[vlc-devel] [PATCH] mediacodec: fix crash with --no-mediacodec-dr

[Steve Lhomme]

On 2021-01-04 9:48, Alexandre Janniaux wrote:
> Hi,
> 
> On Mon, Jan 04, 2021 at 09:38:45AM +0100, Steve Lhomme wrote:
>> This is correct but is not very resilient. ReleaseAllPictureContexts can be
>> called from CloseDecoder > CleanDecoder > StopMediaCodec. When it called the
>> video.ctx has already been freed. It's only because it's not set to NULL
>> that this code would work.
> 
> I'm not sure of what you mean.

You're testing a pointer value that is free'd. This is borderline IMO.

But in fact when the VCTX was not NULL this code is not called.

StopMediaCodec() is called only in CleanDecoder (if VCTX is NULL) or 
during a decoder drain/restart. But the Flush has already called the 
ReleaseAllPictureContexts() so in this case it's called twice ?

>>
>> IMO it would be better to just initialize the table with a NULL VCTX.
> 
> Which table?

video.apic_ctxs, array if you prefer.

> Regards,
> --
> Alexandre Janniaux
> Videolabs
> 
>> On 2021-01-03 16:40, Alexandre Janniaux wrote:
>>> The android picture contexts are not initialized when mediacodec-dr has
>>> been disabled, since it's done at the creation of the video context.
>>> Since we don't have android picture, it's indeed expected that we don't
>>> need to release their context.
>>>
>>> Fix #24698
>>> ---
>>>    modules/codec/omxil/mediacodec.c | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/modules/codec/omxil/mediacodec.c b/modules/codec/omxil/mediacodec.c
>>> index e5dae9343f..ebf3178b38 100644
>>> --- a/modules/codec/omxil/mediacodec.c
>>> +++ b/modules/codec/omxil/mediacodec.c
>>> @@ -571,6 +571,10 @@ static void CleanFromVideoContext(void *priv)
>>>
>>>    static void ReleaseAllPictureContexts(decoder_sys_t *p_sys)
>>>    {
>>> +    /* No picture context if no direct rendering. */
>>> +    if (p_sys->video.ctx == NULL)
>>> +        return;
>>> +
>>>        for (size_t i = 0; i < ARRAY_SIZE(p_sys->video.apic_ctxs); ++i)
>>>        {
>>>            struct android_picture_ctx *apctx = &p_sys->video.apic_ctxs[i];
>>> --
>>> 2.30.0
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>