{"action":"created","comment":{"url":"https://api.github.com/repos/SFML/SFML/pulls/comments/83365454","pull_request_review_id":4214525,"id":83365454,"diff_hunk":"@@ -142,10 +138,12 @@ namespace\n                         break;\n                 }\n \n+                // If there's room in the output buffer, copy the sample there\n                 if (data->remaining > 0)\n                 {\n-                    // There's room in the output buffer, copy the sample there\n-                    *data->buffer++ = sample;\n+                    // If there's no output buffer, it means that we are seeking, so just skip the output write\n+                    if (data->buffer)\n+                        *data->buffer++ = sample;\n                     data->remaining--;","path":"src/SFML/Audio/SoundFileReaderFlac.cpp","position":23,"original_position":23,"commit_id":"cc250327aa0549a9cd6b1bee8b0dc484156a0aaa","original_commit_id":"cc250327aa0549a9cd6b1bee8b0dc484156a0aaa","user":{"login":"LaurentGomila","id":245489,"avatar_url":"https://avatars.githubusercontent.com/u/245489?v=3","gravatar_id":"","url":"https://api.github.com/users/LaurentGomila","html_url":"https://github.com/LaurentGomila","followers_url":"https://api.github.com/users/LaurentGomila/followers","following_url":"https://api.github.com/users/LaurentGomila/following{/other_user}","gists_url":"https://api.github.com/users/LaurentGomila/gists{/gist_id}","starred_url":"https://api.github.com/users/LaurentGomila/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/LaurentGomila/subscriptions","organizations_url":"https://api.github.com/users/LaurentGomila/orgs","repos_url":"https://api.github.com/users/LaurentGomila/repos","events_url":"https://api.github.com/users/LaurentGomila/events{/privacy}","received_events_url":"https://api.github.com/users/LaurentGomila/received_events","type":"User","site_admin":false},"body":"What's the effect of decrementing data->remaining without adding data to the buffer? Well, is it useful at all to execute this statement if there's no buffer?","created_at":"2016-10-14T06:43:43Z","updated_at":"2016-10-14T06:43:43Z","html_url":"https://github.com/SFML/SFML/pull/1162#discussion_r83365454","pull_request_url":"https://api.github.com/repos/SFML/SFML/pulls/1162","_links":{"self":{"href":"https://api.github.com/repos/SFML/SFML/pulls/comments/83365454"},"html":{"href":"https://github.com/SFML/SFML/pull/1162#discussion_r83365454"},"pull_request":{"href":"https://api.github.com/repos/SFML/SFML/pulls/1162"}}},"pull_request":{"url":"https://api.github.com/repos/SFML/SFML/pulls/1162","id":89326309,"html_url":"https://github.com/SFML/SFML/pull/1162","diff_url":"https://github.com/SFML/SFML/pull/1162.diff","patch_url":"https://github.com/SFML/SFML/pull/1162.patch","issue_url":"https://api.github.com/repos/SFML/SFML/issues/1162","number":1162,"state":"open","locked":false,"title":"Addressed Seeking Quirks in FLAC Reader","user":{"login":"Cobaltergeist","id":2526440,"avatar_url":"https://avatars.githubusercontent.com/u/2526440?v=3","gravatar_id":"","url":"https://api.github.com/users/Cobaltergeist","html_url":"https://github.com/Cobaltergeist","followers_url":"https://api.github.com/users/Cobaltergeist/followers","following_url":"https://api.github.com/users/Cobaltergeist/following{/other_user}","gists_url":"https://api.github.com/users/Cobaltergeist/gists{/gist_id}","starred_url":"https://api.github.com/users/Cobaltergeist/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Cobaltergeist/subscriptions","organizations_url":"https://api.github.com/users/Cobaltergeist/orgs","repos_url":"https://api.github.com/users/Cobaltergeist/repos","events_url":"https://api.github.com/users/Cobaltergeist/events{/privacy}","received_events_url":"https://api.github.com/users/Cobaltergeist/received_events","type":"User","site_admin":false},"body":"Here's another fix for a bug I've identified while working on #1118. That PR seemed to fix the behavior of WAV and OGG files, but FLAC was still behaving oddly. I did some digging and figured out why:\r\n\r\nThe problem was that the underlying FLAC decoder has 2 seeking quirks that `SoundFileReaderFlac::seek()` and `streamWrite()` overlook.\r\n\r\nFirst, every seek operation triggers a `streamWrite()` callback. Previously, this case would simply cause the function to return, but it turns out that it actually needs to do some work. When seeking, you'll land mid-block more often than not. And when that happens, the decoder feeds all the samples from the seek destination to the end of its block into `streamWrite()`. Ignoring those bytes is incorrect, as the next read call actually starts at the beginning of the *next* block. By skipping this partial block, we leave the stream at a later point than higher-level trackers (namely `InputSoundFile`) expect it to be. This causes the \"early\" short-read. To fix this I changed `streamWrite()` to act on these seek-decodes, and dump the output samples into the `leftovers` vector, where they will be picked up by the next `SoundFileReaderFlac::read()`.\r\n\r\nSecond, the FLAC decoder does not like seeking straight to EOF. The workaround is easy. I just seek to one sample before the EOF, call `FLAC__stream_decoder_skip_single_frame()`, and re-clear the `leftovers` vector. In the process of fixing this one, I noticed that the `m_channelCount` variable added to address part of #966 is actually redundant. I removed it in favor of the already-available `m_clientData.info.channelCount`.\r\n\r\nTL;DR, FLACs play fine from the start, but if you start jumping around, samples get dropped. And if you try to overrun the file, the underlying decoder resets and you start inadvertently playing sound from the beginning of the stream when you think you're at the end.\r\n\r\nOverall, this is a smaller change, so hopefully it won't be too tough to review. I think I also found a small potential buffer overrun in an obscure condition within `SoundFileReaderFlac::read`. That code seemingly has the potential to overflow `samples` in the unlikely case where the `leftovers` buffer is larger than `maxCount`. It's a tiny tweak to fix, so I went ahead and did that.","created_at":"2016-10-14T05:27:27Z","updated_at":"2016-10-14T06:57:14Z","closed_at":null,"merged_at":null,"merge_commit_sha":"9c8cf831457570378acc8c27a6d43fb81c659fe2","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/SFML/SFML/pulls/1162/commits","review_comments_url":"https://api.github.com/repos/SFML/SFML/pulls/1162/comments","review_comment_url":"https://api.github.com/repos/SFML/SFML/pulls/comments{/number}","comments_url":"https://api.github.com/repos/SFML/SFML/issues/1162/comments","statuses_url":"https://api.github.com/repos/SFML/SFML/statuses/cc250327aa0549a9cd6b1bee8b0dc484156a0aaa","head":{"label":"Cobaltergeist:bugfix/flac_seek","ref":"bugfix/flac_seek","sha":"cc250327aa0549a9cd6b1bee8b0dc484156a0aaa","user":{"login":"Cobaltergeist","id":2526440,"avatar_url":"https://avatars.githubusercontent.com/u/2526440?v=3","gravatar_id":"","url":"https://api.github.com/users/Cobaltergeist","html_url":"https://github.com/Cobaltergeist","followers_url":"https://api.github.com/users/Cobaltergeist/followers","following_url":"https://api.github.com/users/Cobaltergeist/following{/other_user}","gists_url":"https://api.github.com/users/Cobaltergeist/gists{/gist_id}","starred_url":"https://api.github.com/users/Cobaltergeist/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Cobaltergeist/subscriptions","organizations_url":"https://api.github.com/users/Cobaltergeist/orgs","repos_url":"https://api.github.com/users/Cobaltergeist/repos","events_url":"https://api.github.com/users/Cobaltergeist/events{/privacy}","received_events_url":"https://api.github.com/users/Cobaltergeist/received_events","type":"User","site_admin":false},"repo":{"id":20606871,"name":"SFML","full_name":"Cobaltergeist/SFML","owner":{"login":"Cobaltergeist","id":2526440,"avatar_url":"https://avatars.githubusercontent.com/u/2526440?v=3","gravatar_id":"","url":"https://api.github.com/users/Cobaltergeist","html_url":"https://github.com/Cobaltergeist","followers_url":"https://api.github.com/users/Cobaltergeist/followers","following_url":"https://api.github.com/users/Cobaltergeist/following{/other_user}","gists_url":"https://api.github.com/users/Cobaltergeist/gists{/gist_id}","starred_url":"https://api.github.com/users/Cobaltergeist/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Cobaltergeist/subscriptions","organizations_url":"https://api.github.com/users/Cobaltergeist/orgs","repos_url":"https://api.github.com/users/Cobaltergeist/repos","events_url":"https://api.github.com/users/Cobaltergeist/events{/privacy}","received_events_url":"https://api.github.com/users/Cobaltergeist/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/Cobaltergeist/SFML","description":"Simple and Fast Multimedia Library","fork":true,"url":"https://api.github.com/repos/Cobaltergeist/SFML","forks_url":"https://api.github.com/repos/Cobaltergeist/SFML/forks","keys_url":"https://api.github.com/repos/Cobaltergeist/SFML/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Cobaltergeist/SFML/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Cobaltergeist/SFML/teams","hooks_url":"https://api.github.com/repos/Cobaltergeist/SFML/hooks","issue_events_url":"https://api.github.com/repos/Cobaltergeist/SFML/issues/events{/number}","events_url":"https://api.github.com/repos/Cobaltergeist/SFML/events","assignees_url":"https://api.github.com/repos/Cobaltergeist/SFML/assignees{/user}","branches_url":"https://api.github.com/repos/Cobaltergeist/SFML/branches{/branch}","tags_url":"https://api.github.com/repos/Cobaltergeist/SFML/tags","blobs_url":"https://api.github.com/repos/Cobaltergeist/SFML/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Cobaltergeist/SFML/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Cobaltergeist/SFML/git/refs{/sha}","trees_url":"https://api.github.com/repos/Cobaltergeist/SFML/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Cobaltergeist/SFML/statuses/{sha}","languages_url":"https://api.github.com/repos/Cobaltergeist/SFML/languages","stargazers_url":"https://api.github.com/repos/Cobaltergeist/SFML/stargazers","contributors_url":"https://api.github.com/repos/Cobaltergeist/SFML/contributors","subscribers_url":"https://api.github.com/repos/Cobaltergeist/SFML/subscribers","subscription_url":"https://api.github.com/repos/Cobaltergeist/SFML/subscription","commits_url":"https://api.github.com/repos/Cobaltergeist/SFML/commits{/sha}","git_commits_url":"https://api.github.com/repos/Cobaltergeist/SFML/git/commits{/sha}","comments_url":"https://api.github.com/repos/Cobaltergeist/SFML/comments{/number}","issue_comment_url":"https://api.github.com/repos/Cobaltergeist/SFML/issues/comments{/number}","contents_url":"https://api.github.com/repos/Cobaltergeist/SFML/contents/{+path}","compare_url":"https://api.github.com/repos/Cobaltergeist/SFML/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Cobaltergeist/SFML/merges","archive_url":"https://api.github.com/repos/Cobaltergeist/SFML/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Cobaltergeist/SFML/downloads","issues_url":"https://api.github.com/repos/Cobaltergeist/SFML/issues{/number}","pulls_url":"https://api.github.com/repos/Cobaltergeist/SFML/pulls{/number}","milestones_url":"https://api.github.com/repos/Cobaltergeist/SFML/milestones{/number}","notifications_url":"https://api.github.com/repos/Cobaltergeist/SFML/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Cobaltergeist/SFML/labels{/name}","releases_url":"https://api.github.com/repos/Cobaltergeist/SFML/releases{/id}","deployments_url":"https://api.github.com/repos/Cobaltergeist/SFML/deployments","created_at":"2014-06-08T01:42:59Z","updated_at":"2015-04-30T17:23:43Z","pushed_at":"2016-10-14T05:01:55Z","git_url":"git://github.com/Cobaltergeist/SFML.git","ssh_url":"git@github.com:Cobaltergeist/SFML.git","clone_url":"https://github.com/Cobaltergeist/SFML.git","svn_url":"https://github.com/Cobaltergeist/SFML","homepage":"http://www.sfml-dev.org/","size":77739,"stargazers_count":0,"watchers_count":0,"language":"C","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"SFML:master","ref":"master","sha":"2207af41e41141a0afc5cab57ed523679d1befdd","user":{"login":"SFML","id":4226899,"avatar_url":"https://avatars.githubusercontent.com/u/4226899?v=3","gravatar_id":"","url":"https://api.github.com/users/SFML","html_url":"https://github.com/SFML","followers_url":"https://api.github.com/users/SFML/followers","following_url":"https://api.github.com/users/SFML/following{/other_user}","gists_url":"https://api.github.com/users/SFML/gists{/gist_id}","starred_url":"https://api.github.com/users/SFML/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SFML/subscriptions","organizations_url":"https://api.github.com/users/SFML/orgs","repos_url":"https://api.github.com/users/SFML/repos","events_url":"https://api.github.com/users/SFML/events{/privacy}","received_events_url":"https://api.github.com/users/SFML/received_events","type":"Organization","site_admin":false},"repo":{"id":1524684,"name":"SFML","full_name":"SFML/SFML","owner":{"login":"SFML","id":4226899,"avatar_url":"https://avatars.githubusercontent.com/u/4226899?v=3","gravatar_id":"","url":"https://api.github.com/users/SFML","html_url":"https://github.com/SFML","followers_url":"https://api.github.com/users/SFML/followers","following_url":"https://api.github.com/users/SFML/following{/other_user}","gists_url":"https://api.github.com/users/SFML/gists{/gist_id}","starred_url":"https://api.github.com/users/SFML/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SFML/subscriptions","organizations_url":"https://api.github.com/users/SFML/orgs","repos_url":"https://api.github.com/users/SFML/repos","events_url":"https://api.github.com/users/SFML/events{/privacy}","received_events_url":"https://api.github.com/users/SFML/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/SFML/SFML","description":"Simple and Fast Multimedia Library","fork":false,"url":"https://api.github.com/repos/SFML/SFML","forks_url":"https://api.github.com/repos/SFML/SFML/forks","keys_url":"https://api.github.com/repos/SFML/SFML/keys{/key_id}","collaborators_url":"https://api.github.com/repos/SFML/SFML/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/SFML/SFML/teams","hooks_url":"https://api.github.com/repos/SFML/SFML/hooks","issue_events_url":"https://api.github.com/repos/SFML/SFML/issues/events{/number}","events_url":"https://api.github.com/repos/SFML/SFML/events","assignees_url":"https://api.github.com/repos/SFML/SFML/assignees{/user}","branches_url":"https://api.github.com/repos/SFML/SFML/branches{/branch}","tags_url":"https://api.github.com/repos/SFML/SFML/tags","blobs_url":"https://api.github.com/repos/SFML/SFML/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/SFML/SFML/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/SFML/SFML/git/refs{/sha}","trees_url":"https://api.github.com/repos/SFML/SFML/git/trees{/sha}","statuses_url":"https://api.github.com/repos/SFML/SFML/statuses/{sha}","languages_url":"https://api.github.com/repos/SFML/SFML/languages","stargazers_url":"https://api.github.com/repos/SFML/SFML/stargazers","contributors_url":"https://api.github.com/repos/SFML/SFML/contributors","subscribers_url":"https://api.github.com/repos/SFML/SFML/subscribers","subscription_url":"https://api.github.com/repos/SFML/SFML/subscription","commits_url":"https://api.github.com/repos/SFML/SFML/commits{/sha}","git_commits_url":"https://api.github.com/repos/SFML/SFML/git/commits{/sha}","comments_url":"https://api.github.com/repos/SFML/SFML/comments{/number}","issue_comment_url":"https://api.github.com/repos/SFML/SFML/issues/comments{/number}","contents_url":"https://api.github.com/repos/SFML/SFML/contents/{+path}","compare_url":"https://api.github.com/repos/SFML/SFML/compare/{base}...{head}","merges_url":"https://api.github.com/repos/SFML/SFML/merges","archive_url":"https://api.github.com/repos/SFML/SFML/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/SFML/SFML/downloads","issues_url":"https://api.github.com/repos/SFML/SFML/issues{/number}","pulls_url":"https://api.github.com/repos/SFML/SFML/pulls{/number}","milestones_url":"https://api.github.com/repos/SFML/SFML/milestones{/number}","notifications_url":"https://api.github.com/repos/SFML/SFML/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/SFML/SFML/labels{/name}","releases_url":"https://api.github.com/repos/SFML/SFML/releases{/id}","deployments_url":"https://api.github.com/repos/SFML/SFML/deployments","created_at":"2011-03-25T08:05:25Z","updated_at":"2016-10-13T20:22:18Z","pushed_at":"2016-10-14T05:27:28Z","git_url":"git://github.com/SFML/SFML.git","ssh_url":"git@github.com:SFML/SFML.git","clone_url":"https://github.com/SFML/SFML.git","svn_url":"https://github.com/SFML/SFML","homepage":"http://www.sfml-dev.org/","size":97772,"stargazers_count":3084,"watchers_count":3084,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":688,"mirror_url":null,"open_issues_count":82,"forks":688,"open_issues":82,"watchers":3084,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/SFML/SFML/pulls/1162"},"html":{"href":"https://github.com/SFML/SFML/pull/1162"},"issue":{"href":"https://api.github.com/repos/SFML/SFML/issues/1162"},"comments":{"href":"https://api.github.com/repos/SFML/SFML/issues/1162/comments"},"review_comments":{"href":"https://api.github.com/repos/SFML/SFML/pulls/1162/comments"},"review_comment":{"href":"https://api.github.com/repos/SFML/SFML/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/SFML/SFML/pulls/1162/commits"},"statuses":{"href":"https://api.github.com/repos/SFML/SFML/statuses/cc250327aa0549a9cd6b1bee8b0dc484156a0aaa"}}}}
{"actor":{"display_login":"LaurentGomila"}}