diff mbox

[FFmpeg-devel,v2] avcodec/vp8: fix multiple ff_thread_finish_setup() calls

Message ID 20191114042955.96220-1-quinkblack@foxmail.com
State New
Headers show

Commit Message

zhilizhao Nov. 14, 2019, 4:29 a.m. UTC
From: Zhao Zhili <zhilizhao@tencent.com>

webp decoder doesn't set update_thread_context field

$ ffmpeg -i rgb_q80.webp -f null -
[webp @ 0x7ffbd5823200] Multiple ff_thread_finish_setup() calls
---
 libavcodec/vp8.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

zhilizhao Nov. 18, 2019, 2:34 a.m. UTC | #1
Ping for review, thanks!

> On Nov 14, 2019, at 12:29 PM, quinkblack@foxmail.com wrote:
> 
> From: Zhao Zhili <zhilizhao@tencent.com>
> 
> webp decoder doesn't set update_thread_context field
> 
> $ ffmpeg -i rgb_q80.webp -f null -
> [webp @ 0x7ffbd5823200] Multiple ff_thread_finish_setup() calls
> ---
> libavcodec/vp8.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
> index eb51d1f3c9..b4deb3ed67 100644
> --- a/libavcodec/vp8.c
> +++ b/libavcodec/vp8.c
> @@ -2715,7 +2715,8 @@ int vp78_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
> 
>     s->next_framep[VP56_FRAME_CURRENT] = curframe;
> 
> -    ff_thread_finish_setup(avctx);
> +    if (avctx->codec->update_thread_context)
> +        ff_thread_finish_setup(avctx);
> 
>     if (avctx->hwaccel) {
>         ret = avctx->hwaccel->start_frame(avctx, avpkt->data, avpkt->size);
> -- 
> 2.22.0
> 
> 
> 
> _______________________________________________
> 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".
Peter Ross Nov. 18, 2019, 11 a.m. UTC | #2
On Mon, Nov 18, 2019 at 10:34:32AM +0800, zhilizhao wrote:
> Ping for review, thanks!

approve. 还有这块补丁也跟vp7有关,可以在推送前改一下。

> 
> > On Nov 14, 2019, at 12:29 PM, quinkblack@foxmail.com wrote:
> > 
> > From: Zhao Zhili <zhilizhao@tencent.com>
> > 
> > webp decoder doesn't set update_thread_context field
> > 
> > $ ffmpeg -i rgb_q80.webp -f null -
> > [webp @ 0x7ffbd5823200] Multiple ff_thread_finish_setup() calls
> > ---
> > libavcodec/vp8.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
> > index eb51d1f3c9..b4deb3ed67 100644
> > --- a/libavcodec/vp8.c
> > +++ b/libavcodec/vp8.c
> > @@ -2715,7 +2715,8 @@ int vp78_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
> > 
> >     s->next_framep[VP56_FRAME_CURRENT] = curframe;
> > 
> > -    ff_thread_finish_setup(avctx);
> > +    if (avctx->codec->update_thread_context)
> > +        ff_thread_finish_setup(avctx);
> > 
> >     if (avctx->hwaccel) {
> >         ret = avctx->hwaccel->start_frame(avctx, avpkt->data, avpkt->size);
> > -- 
> > 2.22.0
> > 
> > 
> > 
> > _______________________________________________
> > 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".
-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
zhilizhao Nov. 25, 2019, 2:14 p.m. UTC | #3
Please help to merge the patch if it’s acceptable, thanks!

> On Nov 18, 2019, at 7:00 PM, Peter Ross <pross@xvid.org> wrote:
> 
> On Mon, Nov 18, 2019 at 10:34:32AM +0800, zhilizhao wrote:
>> Ping for review, thanks!
> 
> approve. 还有这块补丁也跟vp7有关,可以在推送前改一下。

Since the vp7 decoder doesn’t have multi-thread capabilities, it is not affected. It’s more robust with the check, of course.

vp7因不支持多线程,所以检查与否,功能上不受影响。当然,修改之后,代码看着更合理也更健壮。

> 
>> 
>>> On Nov 14, 2019, at 12:29 PM, quinkblack@foxmail.com wrote:
>>> 
>>> From: Zhao Zhili <zhilizhao@tencent.com>
>>> 
>>> webp decoder doesn't set update_thread_context field
>>> 
>>> $ ffmpeg -i rgb_q80.webp -f null -
>>> [webp @ 0x7ffbd5823200] Multiple ff_thread_finish_setup() calls
>>> ---
>>> libavcodec/vp8.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
>>> index eb51d1f3c9..b4deb3ed67 100644
>>> --- a/libavcodec/vp8.c
>>> +++ b/libavcodec/vp8.c
>>> @@ -2715,7 +2715,8 @@ int vp78_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
>>> 
>>>    s->next_framep[VP56_FRAME_CURRENT] = curframe;
>>> 
>>> -    ff_thread_finish_setup(avctx);
>>> +    if (avctx->codec->update_thread_context)
>>> +        ff_thread_finish_setup(avctx);
>>> 
>>>    if (avctx->hwaccel) {
>>>        ret = avctx->hwaccel->start_frame(avctx, avpkt->data, avpkt->size);
>>> -- 
>>> 2.22.0
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> 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".
> -- Peter
> (A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
> _______________________________________________
> 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".
Peter Ross Nov. 27, 2019, 7:53 a.m. UTC | #4
On Mon, Nov 25, 2019 at 10:14:41PM +0800, zhilizhao wrote:
> Please help to merge the patch if it’s acceptable, thanks!

no code changes, it is acceptable as-is.

>
> > On Nov 18, 2019, at 7:00 PM, Peter Ross <pross@xvid.org> wrote:
> > 
> > On Mon, Nov 18, 2019 at 10:34:32AM +0800, zhilizhao wrote:
> >> Ping for review, thanks!
> > 
> > approve. 还有这块补丁也跟vp7有关,可以在推送前改一下。
> 
> Since the vp7 decoder doesn’t have multi-thread capabilities, it is not affected. It’s more robust with the check, of course.

you are right.
the vp7 decoder calls ff_thread_finish_setup(), but ff_thread_finish_setup checks the context->active_thread_type bitmask before doing anything.

> 
> vp7因不支持多线程,所以检查与否,功能上不受影响。当然,修改之后,代码看着更合理也更健壮。
> 
> > 
> >> 
> >>> On Nov 14, 2019, at 12:29 PM, quinkblack@foxmail.com wrote:
> >>> 
> >>> From: Zhao Zhili <zhilizhao@tencent.com>
> >>> 
> >>> webp decoder doesn't set update_thread_context field
> >>> 
> >>> $ ffmpeg -i rgb_q80.webp -f null -
> >>> [webp @ 0x7ffbd5823200] Multiple ff_thread_finish_setup() calls
> >>> ---
> >>> libavcodec/vp8.c | 3 ++-
> >>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>> 
> >>> diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
> >>> index eb51d1f3c9..b4deb3ed67 100644
> >>> --- a/libavcodec/vp8.c
> >>> +++ b/libavcodec/vp8.c
> >>> @@ -2715,7 +2715,8 @@ int vp78_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
> >>> 
> >>>    s->next_framep[VP56_FRAME_CURRENT] = curframe;
> >>> 
> >>> -    ff_thread_finish_setup(avctx);
> >>> +    if (avctx->codec->update_thread_context)
> >>> +        ff_thread_finish_setup(avctx);
> >>> 
> >>>    if (avctx->hwaccel) {
> >>>        ret = avctx->hwaccel->start_frame(avctx, avpkt->data, avpkt->size);
> >>> -- 
> >>> 2.22.0
> >>> 
> >>> 
> >>> 
> >>> _______________________________________________
> >>> 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".
> > -- Peter
> > (A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
> > _______________________________________________
> > 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".
-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
Michael Niedermayer Nov. 28, 2019, 2:36 p.m. UTC | #5
On Wed, Nov 27, 2019 at 06:53:08PM +1100, Peter Ross wrote:
> On Mon, Nov 25, 2019 at 10:14:41PM +0800, zhilizhao wrote:
> > Please help to merge the patch if it’s acceptable, thanks!
> 
> no code changes, it is acceptable as-is.

will apply

thx

[...]
diff mbox

Patch

diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index eb51d1f3c9..b4deb3ed67 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -2715,7 +2715,8 @@  int vp78_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
 
     s->next_framep[VP56_FRAME_CURRENT] = curframe;
 
-    ff_thread_finish_setup(avctx);
+    if (avctx->codec->update_thread_context)
+        ff_thread_finish_setup(avctx);
 
     if (avctx->hwaccel) {
         ret = avctx->hwaccel->start_frame(avctx, avpkt->data, avpkt->size);