[FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback

["zhilizhao(赵志立)"]

> On Dec 7, 2022, at 00:30, Alessandro Di Nepi <alessandro.dinepi at gmail.com> wrote:
> 
> Got you; giving some context here, and you can find all the details in the ticket #10079 (http://trac.ffmpeg.org/ticket/10079).
> 
> The issue has been introduced with the commit d7f4ad88a0df3c1339e142957bf2c40cd056b8ce.
> This patch basically changed:
> 
> • In the function `videotoolbox_start(AVCodecContext *avctx)`,
> 
> ```
> -    decoder_cb.decompressionOutputRefCon   = avctx;
> +    decoder_cb.decompressionOutputRefCon   = avctx->internal->hwaccel_priv_data;
> ```
> 
> •  The context is retrieved in the function, `videotoolbox_decoder_callback(...)`
> 
> ```
> -    AVCodecContext *avctx = opaque;
> -    VTContext *vtctx = avctx->internal->hwaccel_priv_data;
> +    VTContext *vtctx = opaque;
> ```
> 
> Having said that, I see that when the `videotoolbox_start` is called,
> 
> • `avctx` is not NULL,
> • `avctx->internal->hwaccel_priv_data` is NULL

Did you call av_videotoolbox_default_init() or av_videotoolbox_default_init2()?
I think that’s why avctx->internal->hwaccel_priv_data is NULL. That code path is broken.
Does remove the call of av_videotoolbox_default_init()/av_videotoolbox_default_init2() works for you?

> 
> The first time the `videotoolbox_decoder_callback` is called, `avctx->internal->hwaccel_priv_data` now has a value, so before d7f4ad88a `vtctx` has a value.
> After the change, since `avctx->internal->hwaccel_priv_data` is captured in `video toolbox_start`, is NULL and `vtctx` is also NULL.
> 
> Again, this happens just the first time the callback is called; from the second time, vtctx has a proper value, and everything proceeds as expected.
> 
> I'm willing to change the patch if you think there is a better way, but something needs to be done because the library simply crashes in the current state.
> From what I see from the original change, reverting is not an option.
> 
> Looking forward to hear feedback on this.
> 
> Best Regards
> Alessandro
> On 6 Dec 2022, 7:20 +0200, "zhilizhao(赵志立)" <quinkblack at foxmail.com>, wrote:
>> 
>>> On Dec 5, 2022, at 21:36, Rick Kern <kernrj at gmail.com> wrote:
>>> 
>>> On Sun, Dec 4, 2022 at 12:51 PM Alessandro Di Nepi <
>>> alessandro.dinepi at gmail.com> wrote:
>>> 
>>>> On 4 Dec 2022, 17:01 +0200, FFmpeg development discussions and patches <
>>>> ffmpeg-devel at ffmpeg.org>, wrote:
>>>>> When this happens, does it continue happening, or is it transient? My
>>>> main
>>>>> concern is log spamming.
>>>> Good question: this is just a transient state, so that it won't continue
>>>> happening.
>>>> To give you some context: when the decoding start, the value of `vtctx` is
>>>> captured "too" early so that the first time the callback is called, it's
>>>> still NULL.
>>>> The next time it will have a proper value.
>>>> 
>>> If the code isn't setting a variable in time, that issue should be fixed.
>>> Otherwise the decoder will drop frames.
>> 
>> Yes, null pointer check doesn’t looks like a resolution to a race
>> condition. I’m not sure how the race condition happened in the first
>> place.
>> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".