[vlc-devel] [PATCH 3/3] mediacodec: implementation of MediaCodec direct rendering based on the work by Martin Storsjö.

[Felix Abecassis]

Sorry to bump this thread again but I want to get this over with.
This is my last attempt to try to convince you to have IsExitRequested
moved to the API in order to be able to use it for MediaCodec DR,
otherwise I have another option left to solve the bug but it looks
hacky.

Currently in the core, in DecoderDecodeVideo (src/input/decoder.c) we
have the following code:
   while( (p_pic = p_dec->pf_decode_video( p_dec, &p_block )) )
    {
        vout_thread_t  *p_vout = p_owner->p_vout;
        if( DecoderIsExitRequested( p_dec ) )
        {
            /* It prevent freezing VLC in case of broken decoder */
            vout_ReleasePicture( p_vout, p_pic );
            if( p_block )
                block_Release( p_block );
            break;
        }
[...]
Moving DecoderIsExitRequested to the API would just allow us to make
this check a bit earlier, in the pf_decode_video() call at the top.

You pointed out this won't work if the decoder was not instantiated by
the input (e.g. when transcoding), but I think this is not an issue
because:
- MediaCodec DR cannot support transcode since we need an Android
surface to render to. Only in this mode the bug can happen.
- If by default this mode is disabled we won't break transcoding. It
should be enabled with a command-line flag only.
- We should put a warning in the code to warn other developers not to
copy/paste this piece of code in order to preserve transcoding.

With all these points in mind, do you still object the change to
DecoderIsExitRequested? If yes, I think we might fallback to the other
option (which basically add a maximum number of MediaCodec polling
attempts for the decoding function).

Thanks,

2013/12/30 Felix Abecassis <felix.abecassis at gmail.com>:
> 2013/12/30 Rémi Denis-Courmont <remi at remlab.net>:
>> The way the input is designed currently, the decoder cannot "wait" to get an
>> output buffer. This will always risk deadlocking one way or another. If there
>> is no buffer, it is supposed to discard the current frame, but mostly, the core
>> is supposed to compute a sufficiently large number of output buffers.
> I'm talking about MediaCodec input/output buffers, the core cannot
> influence the allocation of these buffers in any way. We can have as
> few as 6 or 8 MediaCodec output buffers.
> The MediaCodec API is asynchronous, when trying to decode a block we
> have to repeatedly poll MediaCodec with function dequeueInputBuffer
> until we get a valid index
> (http://developer.android.com/reference/android/media/MediaCodec.html#dequeueInputBuffer%28long%29).
> This is why we should not drop the current frame if we failed to get a
> MediaCodec input buffer the first time, otherwise we would drop the
> majority of the frames.
> We can specify an infinite timeout duration but this is a bad idea, if
> the MediaCodec output buffers are not released in the mean time, we
> might never return from dequeueInputBuffer. The MediaCodec output
> buffers are only released when the picture is actually displayed by
> the vout, this is different from the fire-and-forget approach with
> mode 1) where output buffers are immediately released after copying
> the content to a picture_t.
>
>> That said, waiting should only fix lost frames in live playback. I do not
>> really understand how paused state would be special with respect to pictures
>> allocation. Afterall the video output and decoder do not even know about being
>> paused.
> The difference is that when the video is paused the MediaCodec output
> buffers are not released.
>
>> I don't really understand how vlc_object_alive or ExitRequested helps. The
>> decoder thread will be sleeping within your custom picture pool code. The
>> input thread will not be able to wake up the decoder thread after it sets the
>> exiting flag.
> If the decoder was asked to exit, we simply abort the MediaCodec
> polling loop allowing the decoder to terminate.
> Otherwise we will never exit the decoding loop because there is no way
> we will get an available buffer because none will be released.
>
> --
> Félix Abecassis
> http://felix.abecassis.me



-- 
Félix Abecassis
http://felix.abecassis.me