[vlc-devel] [PATCH] input: fix crash when flushing the decoder

[Rémi Denis-Courmont]

Le lundi 11 août 2014, 11:56:42 Felix Abecassis a écrit :
> The crash can occur in the following situation:
> 1) A video decoder is created and starts waiting for the first picture:
>    b_waiting = true, b_first = true
> 
> 2) The first picture is received but the decoder is still in waiting mode:
>    b_waiting = true, b_first = false

I think that's a bug already. The code in input_DecoderWait() does not seem to 
make sense:

    while( p_owner->b_waiting && !p_owner->b_has_data )
        ...

I don't see how p_owner->b_waiting can ever change from true to false while 
the loop iterates. Note that input_DecoderStopWait() is called by the same 
thread (input or demux) that calls input_DecoderWait() !

In my opinion, the real problem is that Wait and StopWait are separate (and 
accordingly b_first and b_waiting are distinct). So the decoder ends up in a 
seemingly invalid intermediate state, b_wait && !b_first, even though the lock 
released between those two function calls.

> 3) A second picture is received, the decoder is now waiting in
> DecoderWaitUnblock().

That function makes the decoder thread wait for the owner thread. I don't see 
the point, but there may be some nasty side effects and non-trivial 
interactions with the clock code.

Even in the other direction, the input thread waits for the decoder thread 
only for crappy buffering time estimation and to release the old vout (if none 
of the active decoder reallocated it).

> 4) From the input thread, an EOF event is received, EsOutChangePosition is
> called and sets the decoder in flushing mode.
> 
> 5) The decoder thread wakes up and exits DecoderWaitUnblock because
> b_flushing is true. However this triggers the assertion in
> DecoderDecodeVideo since we have b_waiting && !b_first.

I think the assertion is valid; it boils down to the intended relation between 
b_waiting and b_first. As far as I understand, the real problem occurs earlier.

-- 
Rémi Denis-Courmont
http://www.remlab.net/