{"action":"created","issue":{"url":"https://api.github.com/repos/SFML/SFML/issues/1118","repository_url":"https://api.github.com/repos/SFML/SFML","labels_url":"https://api.github.com/repos/SFML/SFML/issues/1118/labels{/name}","comments_url":"https://api.github.com/repos/SFML/SFML/issues/1118/comments","events_url":"https://api.github.com/repos/SFML/SFML/issues/1118/events","html_url":"https://github.com/SFML/SFML/pull/1118","id":168483321,"number":1118,"title":"Fixed inconsistent seek behavior in SoundStream","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},"labels":[{"url":"https://api.github.com/repos/SFML/SFML/labels/bug","name":"bug","color":"ff4242"},{"url":"https://api.github.com/repos/SFML/SFML/labels/m:sfml-audio","name":"m:sfml-audio","color":"cccccc"},{"url":"https://api.github.com/repos/SFML/SFML/labels/s:accepted","name":"s:accepted","color":"207de5"}],"state":"open","locked":false,"assignee":{"login":"eXpl0it3r","id":920861,"avatar_url":"https://avatars.githubusercontent.com/u/920861?v=3","gravatar_id":"","url":"https://api.github.com/users/eXpl0it3r","html_url":"https://github.com/eXpl0it3r","followers_url":"https://api.github.com/users/eXpl0it3r/followers","following_url":"https://api.github.com/users/eXpl0it3r/following{/other_user}","gists_url":"https://api.github.com/users/eXpl0it3r/gists{/gist_id}","starred_url":"https://api.github.com/users/eXpl0it3r/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eXpl0it3r/subscriptions","organizations_url":"https://api.github.com/users/eXpl0it3r/orgs","repos_url":"https://api.github.com/users/eXpl0it3r/repos","events_url":"https://api.github.com/users/eXpl0it3r/events{/privacy}","received_events_url":"https://api.github.com/users/eXpl0it3r/received_events","type":"User","site_admin":false},"assignees":[{"login":"eXpl0it3r","id":920861,"avatar_url":"https://avatars.githubusercontent.com/u/920861?v=3","gravatar_id":"","url":"https://api.github.com/users/eXpl0it3r","html_url":"https://github.com/eXpl0it3r","followers_url":"https://api.github.com/users/eXpl0it3r/followers","following_url":"https://api.github.com/users/eXpl0it3r/following{/other_user}","gists_url":"https://api.github.com/users/eXpl0it3r/gists{/gist_id}","starred_url":"https://api.github.com/users/eXpl0it3r/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eXpl0it3r/subscriptions","organizations_url":"https://api.github.com/users/eXpl0it3r/orgs","repos_url":"https://api.github.com/users/eXpl0it3r/repos","events_url":"https://api.github.com/users/eXpl0it3r/events{/privacy}","received_events_url":"https://api.github.com/users/eXpl0it3r/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/SFML/SFML/milestones/15","html_url":"https://github.com/SFML/SFML/milestones/2.4.1","labels_url":"https://api.github.com/repos/SFML/SFML/milestones/15/labels","id":1931542,"number":15,"title":"2.4.1","description":"","creator":{"login":"mantognini","id":693483,"avatar_url":"https://avatars.githubusercontent.com/u/693483?v=3","gravatar_id":"","url":"https://api.github.com/users/mantognini","html_url":"https://github.com/mantognini","followers_url":"https://api.github.com/users/mantognini/followers","following_url":"https://api.github.com/users/mantognini/following{/other_user}","gists_url":"https://api.github.com/users/mantognini/gists{/gist_id}","starred_url":"https://api.github.com/users/mantognini/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mantognini/subscriptions","organizations_url":"https://api.github.com/users/mantognini/orgs","repos_url":"https://api.github.com/users/mantognini/repos","events_url":"https://api.github.com/users/mantognini/events{/privacy}","received_events_url":"https://api.github.com/users/mantognini/received_events","type":"User","site_admin":false},"open_issues":2,"closed_issues":21,"state":"open","created_at":"2016-08-09T07:31:59Z","updated_at":"2016-10-06T21:56:39Z","due_on":null,"closed_at":null},"comments":35,"created_at":"2016-07-30T19:34:03Z","updated_at":"2016-10-11T06:38:59Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/SFML/SFML/pulls/1118","html_url":"https://github.com/SFML/SFML/pull/1118","diff_url":"https://github.com/SFML/SFML/pull/1118.diff","patch_url":"https://github.com/SFML/SFML/pull/1118.patch"},"body":"Sorry for being so spotty with my updates to the Loop Points. One of the things I was tasked to do was isolate my bugfixes for `sf::SoundStream` into a separate PR so that #629 could be decided independently.\r\n\r\nNow, for anyone who doesn't know what this is about, here's a summary of what I addressed in this PR, namely two edge-cases that break the consistency of `m_samplesProcessed` and one regression when `setPlayingOffset()` and `play()` are used when the sound is stopped:\r\n\r\nI only found the first one because I was using a file that was an exact numbers of seconds long, which matches the SoundStream buffer size. Currently, if `onGetData()` reads to the exact end of the file, it will return `true` due to the full read, even though there's no more data available afterward. However, this means that the buffer isn't properly marked as an \"end\". When it's time to read the next buffer after that, it will read 0 bytes and return `false`. This will trigger a seek to 0 (in the current version without loop points), and flag the buffer as an \"end\". But since the read was 0, the buffer then gets refilled with 1 second of post-seek data. The two buffers get played normally, but now the *2nd* one is flagged as the end, even though it contains beginning data. And when it gets unloaded, the sample counter gets reset one second too late. This creates a \"lag\" that persists over the playback, forever reporting an incorrect play position.\r\n\r\nMy fix involved adding `m_tellPos`, in `sf::Music` to keep track of the sample position in the underlying file. Now, if the position afterwards lands on the EOF, `onGetData()` will return false, even if the a read fills the whole buffer. I couldn't fix it in `sf::SoundStream` itself, because modifying the signature of `onSeek()` would break the API. But `sf::Music` is close enough to the underlying file to track its position, and has enough leverage with its return value from `onGetData()` so it can force a zero-seek at the right time.\r\n\r\nA slightly different case involved a similar error when the play position starts at the end-of-file. It causes the same \"delayed zero-read\" effect, though the offset corrects itself after a loop iteration. Since no sound is loaded yet, there's no rightful owner of the \"end\" status, so my solution is the `immediateLoop` parameter in `fillAndPushBuffer()` it's only set to `true` during the `fillQueue()` call as the thread starts up. It essentially gives `fillAndPushBuffer()` permission to immediately adjust `m_samplesProcessed` to the correct post-loop value, instead of deferring things until the refilled buffer gets unloaded a second too late.\r\n\r\nThese fixes combined lead to an included easy-fix of #966, since `sf::Music()` can now clamp a seek overrun to the EOF, invoking the new reset behavior at the next `onGetData()`.\r\n\r\nI'd argue that the last \"bug\" is a regression. `setPlayingOffset()` used to always begin playing the sound when it was called. But when that behavior was removed, the ability to manually start playing afterward from the new offset wasn't added. I found this issue during my Loop Point testing, when I tried to set the \"play start\" by calling `setPlayingOffset()` between `openFromFile()` and `play()`. I noticed that the music always began from the start regardless, due to the `onSeek(Time::Zero);` call in `play()` and I didn't see any proper reason for it. The paused and playing cases were already covered, and if the music was stopped, then there are 3 possibilities. Either the music was newly loaded (already 0), newly-stopped (already 0), or had its offset explicitly adjusted after loading or stopping (why would you overrule that?). So I removed the `onSeek(Time::Zero);` line, and moved the `m_samplesProcessed = 0;` line to `initialize()`, to make sure case 1 is still covered.\r\n\r\nSo that's my summary of the bugs, in isolation from #629. I'll replace the code in that PR with fast-forward from this one soon.\r\n\r\nEDIT: Here's a new [test.cpp](http://pastebin.com/Ae6AyLkW). It's just my newest Loop Point one with the actual loop point calls disabled by a commented-out macro at the top. I still recommend using it with [count.wav](https://dl.dropboxusercontent.com/u/82878780/Misc/count.wav).\r\n\r\n\r\nEDIT by @mantognini: [both the source code and sound file in one archive](https://github.com/SFML/SFML/files/392428/test.material.zip)\r\n"},"comment":{"url":"https://api.github.com/repos/SFML/SFML/issues/comments/252826329","html_url":"https://github.com/SFML/SFML/pull/1118#issuecomment-252826329","issue_url":"https://api.github.com/repos/SFML/SFML/issues/1118","id":252826329,"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},"created_at":"2016-10-11T06:38:59Z","updated_at":"2016-10-11T06:38:59Z","body":"Do we really need to clamp `maxCount` before reading? I mean, if the underlying reader is correctly implemented, it cannot return more samples than what remains until EOF. It feels really weird to handle that at this level. If it's just a matter of fixing one or more reader(s), I'd prefer to focus on them and keep the higher-level code clean."}}
{"actor":{"display_login":"LaurentGomila"}}