* [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
@ 2022-11-27 17:33 Alessandro Di Nepi
2022-11-29 16:46 ` Alessandro Di Nepi
0 siblings, 1 reply; 14+ messages in thread
From: Alessandro Di Nepi @ 2022-11-27 17:33 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Alessandro Di Nepi
The commit d7f4ad88a0df3c1339e142957bf2c40cd056b8ce introduced a race
condition where the passed opaque pointer reference might be NULL,
when the decoding process starts.
This patch checks that vtctx has a value before accessing it.
This patch fixes #10079.
Signed-off-by: Alessandro Di Nepi <alessandro.dinepi@gmail.com>
---
libavcodec/videotoolbox.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
index 1b1be8ddb4..615e2b087a 100644
--- a/libavcodec/videotoolbox.c
+++ b/libavcodec/videotoolbox.c
@@ -692,6 +692,11 @@ static void videotoolbox_decoder_callback(void *opaque,
{
VTContext *vtctx = opaque;
+ if (!vtctx) {
+ av_log(NULL, AV_LOG_WARNING, "vt decoder cb: vt context is null");
+ return;
+ }
+
if (vtctx->frame) {
CVPixelBufferRelease(vtctx->frame);
vtctx->frame = NULL;
--
2.37.1 (Apple Git-137.1)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
2022-11-27 17:33 [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback Alessandro Di Nepi
@ 2022-11-29 16:46 ` Alessandro Di Nepi
2022-12-04 14:02 ` Alessandro Di Nepi
2022-12-04 15:00 ` Rick Kern
0 siblings, 2 replies; 14+ messages in thread
From: Alessandro Di Nepi @ 2022-11-29 16:46 UTC (permalink / raw)
To: ffmpeg-devel
Just to add that this fix, once approved, should be cherry-picked to all the release branches where d7f4ad88a0df3c1339e142957bf2c40cd056b8ce has been cherry-picked.
Basically, 4.4, 5.0, and 5.1.
Thanks
On 27 Nov 2022, 19:34 +0200, Alessandro Di Nepi <alessandro.dinepi@gmail.com>, wrote:
> The commit d7f4ad88a0df3c1339e142957bf2c40cd056b8ce introduced a race
> condition where the passed opaque pointer reference might be NULL,
> when the decoding process starts.
> This patch checks that vtctx has a value before accessing it.
>
> This patch fixes #10079.
>
> Signed-off-by: Alessandro Di Nepi <alessandro.dinepi@gmail.com>
> ---
> libavcodec/videotoolbox.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> index 1b1be8ddb4..615e2b087a 100644
> --- a/libavcodec/videotoolbox.c
> +++ b/libavcodec/videotoolbox.c
> @@ -692,6 +692,11 @@ static void videotoolbox_decoder_callback(void *opaque,
> {
> VTContext *vtctx = opaque;
>
> + if (!vtctx) {
> + av_log(NULL, AV_LOG_WARNING, "vt decoder cb: vt context is null");
> + return;
> + }
> +
> if (vtctx->frame) {
> CVPixelBufferRelease(vtctx->frame);
> vtctx->frame = NULL;
> --
> 2.37.1 (Apple Git-137.1)
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
2022-11-29 16:46 ` Alessandro Di Nepi
@ 2022-12-04 14:02 ` Alessandro Di Nepi
2022-12-04 15:00 ` Rick Kern
1 sibling, 0 replies; 14+ messages in thread
From: Alessandro Di Nepi @ 2022-12-04 14:02 UTC (permalink / raw)
To: ffmpeg-devel
A gentle reminder for this patch: it's a simple fix that prevents crashing on iOS.
Thanks
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
2022-11-29 16:46 ` Alessandro Di Nepi
2022-12-04 14:02 ` Alessandro Di Nepi
@ 2022-12-04 15:00 ` Rick Kern
2022-12-04 17:51 ` Alessandro Di Nepi
1 sibling, 1 reply; 14+ messages in thread
From: Rick Kern @ 2022-12-04 15:00 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Tue, Nov 29, 2022 at 11:47 AM Alessandro Di Nepi <
alessandro.dinepi@gmail.com> wrote:
> Just to add that this fix, once approved, should be cherry-picked to all
> the release branches where d7f4ad88a0df3c1339e142957bf2c40cd056b8ce has
> been cherry-picked.
> Basically, 4.4, 5.0, and 5.1.
>
> Thanks
> On 27 Nov 2022, 19:34 +0200, Alessandro Di Nepi <
> alessandro.dinepi@gmail.com>, wrote:
> > The commit d7f4ad88a0df3c1339e142957bf2c40cd056b8ce introduced a race
> > condition where the passed opaque pointer reference might be NULL,
> > when the decoding process starts.
> > This patch checks that vtctx has a value before accessing it.
> >
> > This patch fixes #10079.
> >
> > Signed-off-by: Alessandro Di Nepi <alessandro.dinepi@gmail.com>
> > ---
> > libavcodec/videotoolbox.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> > index 1b1be8ddb4..615e2b087a 100644
> > --- a/libavcodec/videotoolbox.c
> > +++ b/libavcodec/videotoolbox.c
> > @@ -692,6 +692,11 @@ static void videotoolbox_decoder_callback(void
> *opaque,
> > {
> > VTContext *vtctx = opaque;
> >
> > + if (!vtctx) {
>
When this happens, does it continue happening, or is it transient? My main
concern is log spamming.
> > + av_log(NULL, AV_LOG_WARNING, "vt decoder cb: vt context is null");
> > + return;
> > + }
> > +
> > if (vtctx->frame) {
> > CVPixelBufferRelease(vtctx->frame);
> > vtctx->frame = NULL;
> > --
> > 2.37.1 (Apple Git-137.1)
> >
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
2022-12-04 15:00 ` Rick Kern
@ 2022-12-04 17:51 ` Alessandro Di Nepi
2022-12-05 13:36 ` Rick Kern
0 siblings, 1 reply; 14+ messages in thread
From: Alessandro Di Nepi @ 2022-12-04 17:51 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On 4 Dec 2022, 17:01 +0200, FFmpeg development discussions and patches <ffmpeg-devel@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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
2022-12-04 17:51 ` Alessandro Di Nepi
@ 2022-12-05 13:36 ` Rick Kern
2022-12-06 5:19 ` "zhilizhao(赵志立)"
0 siblings, 1 reply; 14+ messages in thread
From: Rick Kern @ 2022-12-05 13:36 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Sun, Dec 4, 2022 at 12:51 PM Alessandro Di Nepi <
alessandro.dinepi@gmail.com> wrote:
> On 4 Dec 2022, 17:01 +0200, FFmpeg development discussions and patches <
> ffmpeg-devel@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.
_______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
2022-12-05 13:36 ` Rick Kern
@ 2022-12-06 5:19 ` "zhilizhao(赵志立)"
2022-12-06 16:30 ` Alessandro Di Nepi
0 siblings, 1 reply; 14+ messages in thread
From: "zhilizhao(赵志立)" @ 2022-12-06 5:19 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> On Dec 5, 2022, at 21:36, Rick Kern <kernrj@gmail.com> wrote:
>
> On Sun, Dec 4, 2022 at 12:51 PM Alessandro Di Nepi <
> alessandro.dinepi@gmail.com> wrote:
>
>> On 4 Dec 2022, 17:01 +0200, FFmpeg development discussions and patches <
>> ffmpeg-devel@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@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
2022-12-06 5:19 ` "zhilizhao(赵志立)"
@ 2022-12-06 16:30 ` Alessandro Di Nepi
2022-12-09 4:44 ` "zhilizhao(赵志立)"
0 siblings, 1 reply; 14+ messages in thread
From: Alessandro Di Nepi @ 2022-12-06 16:30 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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
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@foxmail.com>, wrote:
>
> > On Dec 5, 2022, at 21:36, Rick Kern <kernrj@gmail.com> wrote:
> >
> > On Sun, Dec 4, 2022 at 12:51 PM Alessandro Di Nepi <
> > alessandro.dinepi@gmail.com> wrote:
> >
> > > On 4 Dec 2022, 17:01 +0200, FFmpeg development discussions and patches <
> > > ffmpeg-devel@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@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
2022-12-06 16:30 ` Alessandro Di Nepi
@ 2022-12-09 4:44 ` "zhilizhao(赵志立)"
2022-12-11 9:57 ` Alessandro Di Nepi
0 siblings, 1 reply; 14+ messages in thread
From: "zhilizhao(赵志立)" @ 2022-12-09 4:44 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> On Dec 7, 2022, at 00:30, Alessandro Di Nepi <alessandro.dinepi@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@foxmail.com>, wrote:
>>
>>> On Dec 5, 2022, at 21:36, Rick Kern <kernrj@gmail.com> wrote:
>>>
>>> On Sun, Dec 4, 2022 at 12:51 PM Alessandro Di Nepi <
>>> alessandro.dinepi@gmail.com> wrote:
>>>
>>>> On 4 Dec 2022, 17:01 +0200, FFmpeg development discussions and patches <
>>>> ffmpeg-devel@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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
2022-12-09 4:44 ` "zhilizhao(赵志立)"
@ 2022-12-11 9:57 ` Alessandro Di Nepi
2022-12-15 14:16 ` Alessandro Di Nepi
0 siblings, 1 reply; 14+ messages in thread
From: Alessandro Di Nepi @ 2022-12-11 9:57 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On 9 Dec 2022, 6:45 +0200, FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>, wrote:
> Did you call av_videotoolbox_default_init() or av_videotoolbox_default_init2()?
Actually yes, I call `av_videotoolbox_default_init(context);`
> 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?
Definitively yes, if I remove the call to `av_videotoolbox_default_init` the context is fine.
Thanks for sorting this out. I'd like to contribute and have this fixed or documented for other people.
What should be the desired behavior? Calling `av_videotoolbox_default_init` and having the context not NULL?
Or shall we remove the init at all?
Best Regards
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
2022-12-11 9:57 ` Alessandro Di Nepi
@ 2022-12-15 14:16 ` Alessandro Di Nepi
2022-12-15 14:45 ` Zhao Zhili
2023-01-09 14:48 ` Zhao Zhili
0 siblings, 2 replies; 14+ messages in thread
From: Alessandro Di Nepi @ 2022-12-15 14:16 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Ping on this,
Can I contribute somehow either to fix calling `av_videotoolbox_default_init` or at least update the docs?
Best Regards,
Alessandro
On 11 Dec 2022, 11:57 +0200, Alessandro Di Nepi <alessandro.dinepi@gmail.com>, wrote:
> On 9 Dec 2022, 6:45 +0200, FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>, wrote:
> > Did you call av_videotoolbox_default_init() or av_videotoolbox_default_init2()?
> Actually yes, I call `av_videotoolbox_default_init(context);`
> > 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?
> Definitively yes, if I remove the call to `av_videotoolbox_default_init` the context is fine.
>
> Thanks for sorting this out. I'd like to contribute and have this fixed or documented for other people.
> What should be the desired behavior? Calling `av_videotoolbox_default_init` and having the context not NULL?
> Or shall we remove the init at all?
>
> Best Regards
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
2022-12-15 14:16 ` Alessandro Di Nepi
@ 2022-12-15 14:45 ` Zhao Zhili
2023-01-09 14:48 ` Zhao Zhili
1 sibling, 0 replies; 14+ messages in thread
From: Zhao Zhili @ 2022-12-15 14:45 UTC (permalink / raw)
To: 'FFmpeg development discussions and patches'
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Alessandro Di Nepi
> Sent: 2022年12月15日 22:16
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
>
> Ping on this,
> Can I contribute somehow either to fix calling `av_videotoolbox_default_init` or at least update the docs?
Updating the docs doesn't work. This is a public API, it must be fixed properly: don't drop a feature, do what
it was supposed to do.
You can give a try, but I'm afraid it's a tricky regression.
>
> Best Regards,
> Alessandro
> On 11 Dec 2022, 11:57 +0200, Alessandro Di Nepi <alessandro.dinepi@gmail.com>, wrote:
> > On 9 Dec 2022, 6:45 +0200, FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>, wrote:
> > > Did you call av_videotoolbox_default_init() or av_videotoolbox_default_init2()?
> > Actually yes, I call `av_videotoolbox_default_init(context);`
> > > 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?
> > Definitively yes, if I remove the call to `av_videotoolbox_default_init` the context is fine.
> >
> > Thanks for sorting this out. I'd like to contribute and have this fixed or documented for other people.
> > What should be the desired behavior? Calling `av_videotoolbox_default_init` and having the context not NULL?
> > Or shall we remove the init at all?
> >
> > Best Regards
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
2022-12-15 14:16 ` Alessandro Di Nepi
2022-12-15 14:45 ` Zhao Zhili
@ 2023-01-09 14:48 ` Zhao Zhili
1 sibling, 0 replies; 14+ messages in thread
From: Zhao Zhili @ 2023-01-09 14:48 UTC (permalink / raw)
To: 'FFmpeg development discussions and patches'
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Alessandro Di Nepi
> Sent: 2022年12月15日 22:16
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
>
> Ping on this,
> Can I contribute somehow either to fix calling `av_videotoolbox_default_init` or at least update the docs?
Hope this patch set fixed the original usecase.
http://ffmpeg.org/pipermail/ffmpeg-devel/2023-January/305477.html
>
> Best Regards,
> Alessandro
> On 11 Dec 2022, 11:57 +0200, Alessandro Di Nepi <alessandro.dinepi@gmail.com>, wrote:
> > On 9 Dec 2022, 6:45 +0200, FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>, wrote:
> > > Did you call av_videotoolbox_default_init() or av_videotoolbox_default_init2()?
> > Actually yes, I call `av_videotoolbox_default_init(context);`
> > > 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?
> > Definitively yes, if I remove the call to `av_videotoolbox_default_init` the context is fine.
> >
> > Thanks for sorting this out. I'd like to contribute and have this fixed or documented for other people.
> > What should be the desired behavior? Calling `av_videotoolbox_default_init` and having the context not NULL?
> > Or shall we remove the init at all?
> >
> > Best Regards
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback
[not found] <53dafc80-d9f6-4b5b-a7a5-781bbb045493@Spark>
@ 2022-11-27 16:21 ` Alessandro Di Nepi
0 siblings, 0 replies; 14+ messages in thread
From: Alessandro Di Nepi @ 2022-11-27 16:21 UTC (permalink / raw)
To: ffmpeg-devel
The commit d7f4ad88a0df3c1339e142957bf2c40cd056b8ce introduced a race
condition where the passed opaque pointer reference might be NULL,
when the decoding process starts.
This patch checks that vtctx has a value before accessing it.
This patch fixes #10079.
Signed-off-by: Alessandro Di Nepi <alessandro.dinepi@gmail.com>
---
libavcodec/videotoolbox.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
index 1b1be8ddb4..615e2b087a 100644
--- a/libavcodec/videotoolbox.c
+++ b/libavcodec/videotoolbox.c
@@ -692,6 +692,11 @@ static void videotoolbox_decoder_callback(void *opaque,
{
VTContext *vtctx = opaque;
+ if (!vtctx) {
+ av_log(NULL, AV_LOG_WARNING, "vt decoder cb: vt context is null");
+ return;
+ }
+
if (vtctx->frame) {
CVPixelBufferRelease(vtctx->frame);
vtctx->frame = NULL;
--
2.37.1 (Apple Git-137.1)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-01-09 14:48 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-27 17:33 [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback Alessandro Di Nepi
2022-11-29 16:46 ` Alessandro Di Nepi
2022-12-04 14:02 ` Alessandro Di Nepi
2022-12-04 15:00 ` Rick Kern
2022-12-04 17:51 ` Alessandro Di Nepi
2022-12-05 13:36 ` Rick Kern
2022-12-06 5:19 ` "zhilizhao(赵志立)"
2022-12-06 16:30 ` Alessandro Di Nepi
2022-12-09 4:44 ` "zhilizhao(赵志立)"
2022-12-11 9:57 ` Alessandro Di Nepi
2022-12-15 14:16 ` Alessandro Di Nepi
2022-12-15 14:45 ` Zhao Zhili
2023-01-09 14:48 ` Zhao Zhili
[not found] <53dafc80-d9f6-4b5b-a7a5-781bbb045493@Spark>
2022-11-27 16:21 ` Alessandro Di Nepi
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git