* [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
* [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 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
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 -- [not found] <53dafc80-d9f6-4b5b-a7a5-781bbb045493@Spark> 2022-11-27 16:21 ` [FFmpeg-devel] [PATCH] lavc/videotoolbox: validate vt context in the decoder callback Alessandro Di Nepi 2022-11-27 17:33 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
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