{"action":"created","comment":{"url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/pulls/comments/175980582","pull_request_review_id":105602336,"id":175980582,"diff_hunk":"@@ -48,28 +54,27 @@ AudioGeneratorMP3::AudioGeneratorMP3(void *space, int size)\n AudioGeneratorMP3::~AudioGeneratorMP3()\n {\n   if (!preallocateSpace) {\n-    free(buff);\n-    free(synth);\n-    free(frame);\n-    free(stream);\n+    if (buff) free(buff);","path":"src/AudioGeneratorMP3.cpp","position":28,"original_position":28,"commit_id":"6f02324a98f43a54b0bbf14e6ed87309b6f9d66b","original_commit_id":"6f02324a98f43a54b0bbf14e6ed87309b6f9d66b","user":{"login":"earlephilhower","id":11875,"avatar_url":"https://avatars0.githubusercontent.com/u/11875?v=4","gravatar_id":"","url":"https://api.github.com/users/earlephilhower","html_url":"https://github.com/earlephilhower","followers_url":"https://api.github.com/users/earlephilhower/followers","following_url":"https://api.github.com/users/earlephilhower/following{/other_user}","gists_url":"https://api.github.com/users/earlephilhower/gists{/gist_id}","starred_url":"https://api.github.com/users/earlephilhower/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/earlephilhower/subscriptions","organizations_url":"https://api.github.com/users/earlephilhower/orgs","repos_url":"https://api.github.com/users/earlephilhower/repos","events_url":"https://api.github.com/users/earlephilhower/events{/privacy}","received_events_url":"https://api.github.com/users/earlephilhower/received_events","type":"User","site_admin":false},"body":"No need for this check.  free(NULL) is perfectly legal and guaranteed to do nothing.","created_at":"2018-03-21T03:59:15Z","updated_at":"2018-03-21T04:05:02Z","html_url":"https://github.com/earlephilhower/ESP8266Audio/pull/76#discussion_r175980582","pull_request_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/pulls/76","author_association":"OWNER","_links":{"self":{"href":"https://api.github.com/repos/earlephilhower/ESP8266Audio/pulls/comments/175980582"},"html":{"href":"https://github.com/earlephilhower/ESP8266Audio/pull/76#discussion_r175980582"},"pull_request":{"href":"https://api.github.com/repos/earlephilhower/ESP8266Audio/pulls/76"}}},"pull_request":{"url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/pulls/76","id":176079798,"html_url":"https://github.com/earlephilhower/ESP8266Audio/pull/76","diff_url":"https://github.com/earlephilhower/ESP8266Audio/pull/76.diff","patch_url":"https://github.com/earlephilhower/ESP8266Audio/pull/76.patch","issue_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/issues/76","number":76,"state":"open","locked":false,"title":"Adding loop support and minimizing artifacts when playing new sounds","user":{"login":"rfestag","id":2946462,"avatar_url":"https://avatars1.githubusercontent.com/u/2946462?v=4","gravatar_id":"","url":"https://api.github.com/users/rfestag","html_url":"https://github.com/rfestag","followers_url":"https://api.github.com/users/rfestag/followers","following_url":"https://api.github.com/users/rfestag/following{/other_user}","gists_url":"https://api.github.com/users/rfestag/gists{/gist_id}","starred_url":"https://api.github.com/users/rfestag/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rfestag/subscriptions","organizations_url":"https://api.github.com/users/rfestag/orgs","repos_url":"https://api.github.com/users/rfestag/repos","events_url":"https://api.github.com/users/rfestag/events{/privacy}","received_events_url":"https://api.github.com/users/rfestag/received_events","type":"User","site_admin":false},"body":"This is a work in progress, but I thought this would be the best way to get feedback on how it should work before going too much further down the rabbit hole. In a nutshell, this code seems to work well for looping the same file over and over (any discontinuity appears to be as a result of the end of the file not quite matching up with the beginning). However, it doesn't handle changing playback to a new file well. The only way I currently have to detect it is to wait until mp3->loop() returns false, and then start a new file. This does seem to work better than it did, but you do still hear some artifacts.\r\n\r\nFirst off, does this seem to be a reasonable solution for looping? In the new code, I basically have a loop flag that, if the AudioGenerator::Input() method returns fewer bytes than expected, we assume EOF and reset the pointer, then re-try calling Input(). It might be possible for this to be recursive more than one level, although I doubt it in practice. Perhaps that logic should be moved to the loop() method though to avoid that (although if it can occur, I think you'll end up in an infinite loop, vs. eventually crashing because the stack fills up, potentially making it easier to see/resolve the problem if it happens).\r\n\r\nSecondly, I feel like I can implement similar logic, but support different files (not looping the same file). This seems to imply allowing at least one file be queued up to immediately run. This basic logic could be used to implement replay (set the \"next\" file to the same as the current file). However, it begs the question of whether we would want to support a list of files of some sort (basically add support to the MP3 generator and possibly others to have a linked list of files to traverse). It also starts to add \"play list\" support directly to the generator, which may not be ideal (although I'm not sure how to get around that and still be able to fill the buffer when Input() is called with contents of the next file to play). Perhaps I could add a callback that fires when a file has been read to completion which would allow a user to dynamically determine what to do next?","created_at":"2018-03-20T03:23:43Z","updated_at":"2018-03-21T04:05:02Z","closed_at":null,"merged_at":null,"merge_commit_sha":"dbcc64b4f51c62bfb86974bfc7e35b3e375678e9","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/pulls/76/commits","review_comments_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/pulls/76/comments","review_comment_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/pulls/comments{/number}","comments_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/issues/76/comments","statuses_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/statuses/6f02324a98f43a54b0bbf14e6ed87309b6f9d66b","head":{"label":"rfestag:master","ref":"master","sha":"6f02324a98f43a54b0bbf14e6ed87309b6f9d66b","user":{"login":"rfestag","id":2946462,"avatar_url":"https://avatars1.githubusercontent.com/u/2946462?v=4","gravatar_id":"","url":"https://api.github.com/users/rfestag","html_url":"https://github.com/rfestag","followers_url":"https://api.github.com/users/rfestag/followers","following_url":"https://api.github.com/users/rfestag/following{/other_user}","gists_url":"https://api.github.com/users/rfestag/gists{/gist_id}","starred_url":"https://api.github.com/users/rfestag/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rfestag/subscriptions","organizations_url":"https://api.github.com/users/rfestag/orgs","repos_url":"https://api.github.com/users/rfestag/repos","events_url":"https://api.github.com/users/rfestag/events{/privacy}","received_events_url":"https://api.github.com/users/rfestag/received_events","type":"User","site_admin":false},"repo":{"id":125951470,"name":"ESP8266Audio","full_name":"rfestag/ESP8266Audio","owner":{"login":"rfestag","id":2946462,"avatar_url":"https://avatars1.githubusercontent.com/u/2946462?v=4","gravatar_id":"","url":"https://api.github.com/users/rfestag","html_url":"https://github.com/rfestag","followers_url":"https://api.github.com/users/rfestag/followers","following_url":"https://api.github.com/users/rfestag/following{/other_user}","gists_url":"https://api.github.com/users/rfestag/gists{/gist_id}","starred_url":"https://api.github.com/users/rfestag/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rfestag/subscriptions","organizations_url":"https://api.github.com/users/rfestag/orgs","repos_url":"https://api.github.com/users/rfestag/repos","events_url":"https://api.github.com/users/rfestag/events{/privacy}","received_events_url":"https://api.github.com/users/rfestag/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/rfestag/ESP8266Audio","description":"Arduino library to play MOD, WAV, FLAC, MIDI, MP3, and AAC files on I2S DACs or with a software emulated delta-sigma DAC on the ESP8266 and ESP32","fork":true,"url":"https://api.github.com/repos/rfestag/ESP8266Audio","forks_url":"https://api.github.com/repos/rfestag/ESP8266Audio/forks","keys_url":"https://api.github.com/repos/rfestag/ESP8266Audio/keys{/key_id}","collaborators_url":"https://api.github.com/repos/rfestag/ESP8266Audio/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/rfestag/ESP8266Audio/teams","hooks_url":"https://api.github.com/repos/rfestag/ESP8266Audio/hooks","issue_events_url":"https://api.github.com/repos/rfestag/ESP8266Audio/issues/events{/number}","events_url":"https://api.github.com/repos/rfestag/ESP8266Audio/events","assignees_url":"https://api.github.com/repos/rfestag/ESP8266Audio/assignees{/user}","branches_url":"https://api.github.com/repos/rfestag/ESP8266Audio/branches{/branch}","tags_url":"https://api.github.com/repos/rfestag/ESP8266Audio/tags","blobs_url":"https://api.github.com/repos/rfestag/ESP8266Audio/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/rfestag/ESP8266Audio/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/rfestag/ESP8266Audio/git/refs{/sha}","trees_url":"https://api.github.com/repos/rfestag/ESP8266Audio/git/trees{/sha}","statuses_url":"https://api.github.com/repos/rfestag/ESP8266Audio/statuses/{sha}","languages_url":"https://api.github.com/repos/rfestag/ESP8266Audio/languages","stargazers_url":"https://api.github.com/repos/rfestag/ESP8266Audio/stargazers","contributors_url":"https://api.github.com/repos/rfestag/ESP8266Audio/contributors","subscribers_url":"https://api.github.com/repos/rfestag/ESP8266Audio/subscribers","subscription_url":"https://api.github.com/repos/rfestag/ESP8266Audio/subscription","commits_url":"https://api.github.com/repos/rfestag/ESP8266Audio/commits{/sha}","git_commits_url":"https://api.github.com/repos/rfestag/ESP8266Audio/git/commits{/sha}","comments_url":"https://api.github.com/repos/rfestag/ESP8266Audio/comments{/number}","issue_comment_url":"https://api.github.com/repos/rfestag/ESP8266Audio/issues/comments{/number}","contents_url":"https://api.github.com/repos/rfestag/ESP8266Audio/contents/{+path}","compare_url":"https://api.github.com/repos/rfestag/ESP8266Audio/compare/{base}...{head}","merges_url":"https://api.github.com/repos/rfestag/ESP8266Audio/merges","archive_url":"https://api.github.com/repos/rfestag/ESP8266Audio/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/rfestag/ESP8266Audio/downloads","issues_url":"https://api.github.com/repos/rfestag/ESP8266Audio/issues{/number}","pulls_url":"https://api.github.com/repos/rfestag/ESP8266Audio/pulls{/number}","milestones_url":"https://api.github.com/repos/rfestag/ESP8266Audio/milestones{/number}","notifications_url":"https://api.github.com/repos/rfestag/ESP8266Audio/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/rfestag/ESP8266Audio/labels{/name}","releases_url":"https://api.github.com/repos/rfestag/ESP8266Audio/releases{/id}","deployments_url":"https://api.github.com/repos/rfestag/ESP8266Audio/deployments","created_at":"2018-03-20T02:50:39Z","updated_at":"2018-03-20T02:54:50Z","pushed_at":"2018-03-20T02:54:49Z","git_url":"git://github.com/rfestag/ESP8266Audio.git","ssh_url":"git@github.com:rfestag/ESP8266Audio.git","clone_url":"https://github.com/rfestag/ESP8266Audio.git","svn_url":"https://github.com/rfestag/ESP8266Audio","homepage":"","size":3435,"stargazers_count":0,"watchers_count":0,"language":"C","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"gpl-3.0","name":"GNU General Public License v3.0","spdx_id":"GPL-3.0","url":"https://api.github.com/licenses/gpl-3.0"},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"earlephilhower:master","ref":"master","sha":"9598c4862ec99e6c2a1979696ca252eb40963794","user":{"login":"earlephilhower","id":11875,"avatar_url":"https://avatars0.githubusercontent.com/u/11875?v=4","gravatar_id":"","url":"https://api.github.com/users/earlephilhower","html_url":"https://github.com/earlephilhower","followers_url":"https://api.github.com/users/earlephilhower/followers","following_url":"https://api.github.com/users/earlephilhower/following{/other_user}","gists_url":"https://api.github.com/users/earlephilhower/gists{/gist_id}","starred_url":"https://api.github.com/users/earlephilhower/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/earlephilhower/subscriptions","organizations_url":"https://api.github.com/users/earlephilhower/orgs","repos_url":"https://api.github.com/users/earlephilhower/repos","events_url":"https://api.github.com/users/earlephilhower/events{/privacy}","received_events_url":"https://api.github.com/users/earlephilhower/received_events","type":"User","site_admin":false},"repo":{"id":100118525,"name":"ESP8266Audio","full_name":"earlephilhower/ESP8266Audio","owner":{"login":"earlephilhower","id":11875,"avatar_url":"https://avatars0.githubusercontent.com/u/11875?v=4","gravatar_id":"","url":"https://api.github.com/users/earlephilhower","html_url":"https://github.com/earlephilhower","followers_url":"https://api.github.com/users/earlephilhower/followers","following_url":"https://api.github.com/users/earlephilhower/following{/other_user}","gists_url":"https://api.github.com/users/earlephilhower/gists{/gist_id}","starred_url":"https://api.github.com/users/earlephilhower/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/earlephilhower/subscriptions","organizations_url":"https://api.github.com/users/earlephilhower/orgs","repos_url":"https://api.github.com/users/earlephilhower/repos","events_url":"https://api.github.com/users/earlephilhower/events{/privacy}","received_events_url":"https://api.github.com/users/earlephilhower/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/earlephilhower/ESP8266Audio","description":"Arduino library to play MOD, WAV, FLAC, MIDI, MP3, and AAC files on I2S DACs or with a software emulated delta-sigma DAC on the ESP8266 and ESP32","fork":false,"url":"https://api.github.com/repos/earlephilhower/ESP8266Audio","forks_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/forks","keys_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/keys{/key_id}","collaborators_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/teams","hooks_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/hooks","issue_events_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/issues/events{/number}","events_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/events","assignees_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/assignees{/user}","branches_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/branches{/branch}","tags_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/tags","blobs_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/git/refs{/sha}","trees_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/git/trees{/sha}","statuses_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/statuses/{sha}","languages_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/languages","stargazers_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/stargazers","contributors_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/contributors","subscribers_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/subscribers","subscription_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/subscription","commits_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/commits{/sha}","git_commits_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/git/commits{/sha}","comments_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/comments{/number}","issue_comment_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/issues/comments{/number}","contents_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/contents/{+path}","compare_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/compare/{base}...{head}","merges_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/merges","archive_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/downloads","issues_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/issues{/number}","pulls_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/pulls{/number}","milestones_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/milestones{/number}","notifications_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/labels{/name}","releases_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/releases{/id}","deployments_url":"https://api.github.com/repos/earlephilhower/ESP8266Audio/deployments","created_at":"2017-08-12T14:47:25Z","updated_at":"2018-03-18T08:30:43Z","pushed_at":"2018-03-20T03:23:43Z","git_url":"git://github.com/earlephilhower/ESP8266Audio.git","ssh_url":"git@github.com:earlephilhower/ESP8266Audio.git","clone_url":"https://github.com/earlephilhower/ESP8266Audio.git","svn_url":"https://github.com/earlephilhower/ESP8266Audio","homepage":"","size":3446,"stargazers_count":82,"watchers_count":82,"language":"C","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":21,"mirror_url":null,"archived":false,"open_issues_count":20,"license":{"key":"gpl-3.0","name":"GNU General Public License v3.0","spdx_id":"GPL-3.0","url":"https://api.github.com/licenses/gpl-3.0"},"forks":21,"open_issues":20,"watchers":82,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/earlephilhower/ESP8266Audio/pulls/76"},"html":{"href":"https://github.com/earlephilhower/ESP8266Audio/pull/76"},"issue":{"href":"https://api.github.com/repos/earlephilhower/ESP8266Audio/issues/76"},"comments":{"href":"https://api.github.com/repos/earlephilhower/ESP8266Audio/issues/76/comments"},"review_comments":{"href":"https://api.github.com/repos/earlephilhower/ESP8266Audio/pulls/76/comments"},"review_comment":{"href":"https://api.github.com/repos/earlephilhower/ESP8266Audio/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/earlephilhower/ESP8266Audio/pulls/76/commits"},"statuses":{"href":"https://api.github.com/repos/earlephilhower/ESP8266Audio/statuses/6f02324a98f43a54b0bbf14e6ed87309b6f9d66b"}},"author_association":"NONE"}}
{"actor":{"display_login":"earlephilhower"}}