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-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to