GNOME Bugzilla – Bug 752431
mpg123audiodec: fix handling of sample rate change during playback
Last modified: 2015-08-17 10:53:23 UTC
Created attachment 307497 [details] [review] Proposed fix I mentioned this in IRC and was able to capture a stream (I'm not sure I can attach it because it is a broadcast and I don't know the legalities of doing so) that exhibited the problem. The issue appears to be that, if the rate changes mid stream, the parser calls the decoder's set_format, and has_next_audioinfo is set to TRUE. This is good. Next, the decoder flushes all frames (also good), and sets has_next_audioinfo to FALSE. This is bad, because the handle_frame logic fails to set the output format, despite having a new format. I've attached a proposed patch. At first I thought this may be only useful for testing, but, looking at the lifecycle of next_audioinfo, I think is valid. next_audioinfo is not cleared during a flush, and since this flag is bound logically to that information, it shouldn't be either. That said, it's (very) likely I've overlooked something, so feedback would be great. I apologize for the lack of unit test, if I can generate an mp3 file of public domain audio that exhibits the issue I will do so. I reproduced this with 1.2.4 and 1.4.5 using the mpg123audiodec. I have not yet tried master, but, the offending line of code still exists, so the problem might be there?
Not sure I fully understand how this is supposed to work exactly, but the patch clearly makes this work properly, so I've merged this for now: commit 8e488e5e490f0eb0bf6573fc7c9d557428ba8e7d Author: Jason Litzinger <jlitzinger@control4.com> Date: Wed Jul 15 10:44:02 2015 -0600 mpg123: fix handling of sample rate change during playback If the sample rate of the media changes, the resulting flush will clear the has_next_audioinfo flag, and the caps won't be sent downstream. https://bugzilla.gnome.org/show_bug.cgi?id=752431 Just after I pushed it, it occured to me that the purpose of the has_next_audioinfo=FALSE in the flush function was probably to reset it in case of a seek, for example. So on second thought, perhaps we should still reset it to FALSE if 'hard' is TRUE, and otherwise not: commit f3b18a29bf4c11bfb6609fb8e029af3603bb1030 Author: Tim-Philipp Müller <tim@centricular.com> Date: Mon Aug 17 11:50:28 2015 +0100 mpg123: still reset pending audio info on hard flush Follow-up to previous commit. https://bugzilla.gnome.org/show_bug.cgi?id=752431 This still seems to make the original issue work as well.