Re: [FFmpeg-devel] [PATCH] lavc/webp: Remove frame threading

[Thilo Borgmann via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

Am 22.10.23 um 16:30 schrieb Andreas Rheinhardt:
> Thilo Borgmann via ffmpeg-devel:
>> Revealed by the patch to support animated webp, the current
>> frame threading implementation contains a data race.
> 
> No, it doesn't: The current implementation does not call
> ff_thread_finish_setup() in vp8.c for webp:
> 
>      if (ffcodec(avctx->codec)->update_thread_context)
>          ff_thread_finish_setup(avctx);
> 
> It seems that "the patch to support animated webp" (what patch are we
> talking about?) adds an update_thread_context to the webp decoder,
> thereby changing things and adding the data race.

Oh yes that's true, no update_thread_context(), so not happening.


>> vp8_lossy_decode_frame() calls ff_vp8_decode_frame() wich
>> calls ff_thread_finish_setup() to sync its internal slice threading.
> 
> Nonsense: ff_thread_finish_setup() is only for frame-threading.

Indeed, thx. Though, even if I was too stupid to realize for what, it is called...


>> The race is happens because vp8_lossy_decode_frame() has to touch
>> the AVCodecContext after it was passed to ff_vp8_decode_frame() and
>> ff_thread_finish_setup() had been called.
>>
>> Therefore remove frame threading in webp and rely on slice threading
>> in VP8 only.
> 
> Also nonsense: The webp decoder does not support slice threading, so the
> internal VP8 decoder won't ever use it (even though it supports it).

Thx again, that's good to know.


> I am a bit confused here: On the one hand,
> https://developers.google.com/speed/webp/docs/riff_container says that
> it only uses VP8 key frame encoding; on the other hand, it has this
> animation feature. Does this also only use VP8-intra coding (i.e. is the
> non-intra part of animation just the blending of earlier frames?)?

That's how it works.

> If it
> does, then the webp decoder should be separated from the VP8 decoder
> (i.e. it should use it according to the public API) and the sub-decoder
> should only be used in single-threaded mode.

That would solve the issue.
How do I force the VP8 decoder to be single-threaded from the API?
Tried that and failed - well, you know how stupid I am...
Agreed, the internal VP8 decoding for lossless images should be gone from webp.c.


> IMO removing frame-threading for ordinary WebP due to animated WebP is
> unacceptable.

Once VP8 decoding happens single-threaded, this patch is void of course.

Thanks,
Thilo
_______________________________________________
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".