GNOME Bugzilla – Bug 791269
audiorate: reset offset and next_ts if caps is changed
Last modified: 2018-07-19 22:34:12 UTC
Created attachment 365028 [details] [review] audiorate: reset offset and next_ts if caps is changed When sink caps is changed, next_offset and next_ts should be re-calculated based on audio information from the caps. Otherwise, samples might be dropped if the rate is down.
any advice for the patch?
As far as I can tell from looking at the chain method, setting these to -1 will cause audiorate to mark itself as discont, and potentially go through the if (skip_to_first) condition, that doesn't seem optimal, couldn't you rather recalculate next_offset and next_ts directly ?
A small unit test would be great as well if you're up for it :)
Created attachment 366533 [details] [review] audiorate: accumulate offset by time diff audiorate: accumulate offset by time diff The fomula, 'offset = time / rate', is correct only if the rate is never changed. When the rate is changed, the offset should be re-calculated based on the previous offset.
Created attachment 366534 [details] [review] audiorate: accumulate offset by time diff
Review of attachment 366534 [details] [review]: I've noticed that this probably won't work with segment->rate < 0, but I think this use-case is already broken with the current code and I think I should write some unit tests for this.
audiorate is already broken for reverse playback, I tried to fix it, but got bored, so I merged hit more or less as-is. commit 4fa850e3e6c039000fc7f648de238af6c2278469 (HEAD -> master, upstream/master) Author: Justin Kim <justin.kim@collabora.com> Date: Fri Jan 5 16:07:54 2018 +0900 audiorate: accumulate offset by time diff The fomula, 'offset = time / rate', is correct only if the rate is never changed. When the rate is changed, the offset should be re-calculated based on the previous offset. https://bugzilla.gnome.org/show_bug.cgi?id=791269