[MPlayer-dev-eng] [PATCH] Bug fix in af_add/del/switch/clr

[Jehan Pagès]

Hi,

On Sun, Jan 23, 2011 at 11:06 PM, Clément Bœsch <ubitux at gmail.com> wrote:
> On Wed, Jan 19, 2011 at 02:29:03PM +0900, Jehan Pagès wrote:
>> Hi,
>>
>
> Hi,
>
>> I have been recently noticed by a user of a bug with audio filter
>> loading-on-the-run commands. Some audio frames were lost, which was
>> unnoticed when used punctually, but when loading/reloading/clearing a
>> lot filters in some time span, more frames lost became noticeable,
>> hence speed was accelerating. And consequently some videos were not
>> resyncing well, so images and audios got desynchronized.
>>
>> Though it seems unlikely to load filters every few milliseconds to
>> notice this, the user who reported the bug used an audio equalizer
>> filter reloading. I guess such an equalizer could be very often
>> reloaded by a mplayer GUI while a user is trying to find the right
>> values to apply to each frequency band (I guess this person was indeed
>> developping some GUI to mplayer). And he wants to do this without
>> music speeding or video desyncing. And anyway that's a bug, so it is
>> good to fix. :-)
>> After test, the problem occurred with any audio filter anyway.
>>
>> -------------------
>> To reproduce:
>>
>> $ mkinfo /tmp/fifompdev
>    ^^^^^^
>     mkfifo I suppose?

Indeed. Typing my email too fast, I guess. :-)

>> $ mplayer  -slave -input file=/tmp/fifompdev <your_media_file>
>> $ while true ; do echo af_switch equalizer=0:0:0:0:0:0:0:0:1:1
>> >/tmp/fifompdev; sleep 0.1; done
>>
>> What happens and should not: in an audio file, the music will
>> accelerate. In a video file, audio and video may desync.
>
> I don't really have a desync but much more sound artifacts.
>

Yes I had too, this is related, because if audio frames are "lost",
that sounds logical to get sound artifacts (even more if you don't
have the desync, I would say).

>> -------------------
>>
>> Basically to fix, I removed calls to reinit_audio_chain(); after such
>> a dynamic load/unload of a filter. This was causing the issue and this
>> was an unecessary call actually as any sound reinitialization was
>> already dealed with in the functions which actually loaded/unloaded
>> audio filters.
>>
>
> While it may be actually uneeded, I don't understand why it is harmful.
>
>> Attached is the small patch file.
>> You can try the bug reproduction before and after, and confirms it has
>> been fixed (I confirmed it on my machine and the user who reported me
>> the bug also did for him).
>
> Yes, I can confirm it fixes the issue.
>
> Btw, I'm not qualified to make a proper review, I can only state the fact
> that well, it works. So whenever a maintainer gives his ok, I'll commit
> it. Until then, I can't do anything.
>

Ok. Thanks! Now let's wait for maintainer's approval then! :-)

Jehan