Am 21.06.24 um 13:52 schrieb Anton Khirnov:
Quoting Thilo Borgmann via ffmpeg-devel (2024-06-21 12:43:17)
From: Thilo Borgmann via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>
---
libavcodec/webp.c | 50 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 44 insertions(+), 6 deletions(-)
diff --git a/libavcodec/webp.c b/libavcodec/webp.c
index af4ebcec27..c52b9732b4 100644
--- a/libavcodec/webp.c
+++ b/libavcodec/webp.c
@@ -195,6 +195,7 @@ typedef struct WebPContext {
AVFrame *alpha_frame; /* AVFrame for alpha data
decompressed from VP8L */
AVPacket *pkt; /* AVPacket to be passed to the
underlying VP8 decoder */
AVCodecContext *avctx; /* parent AVCodecContext */
+ AVCodecContext *avctx_vp8; /* wrapper context for VP8 decoder */
As I said before, nested decoders should be avoided whenever possible,
because properly forwarding everything between the caller and the nested
decoder is very tricky and almost never fully correct. For example, this
patch seems to break direct rendering.
How could I test that?
And even if a nested decoder is unavoidable for some reason, it need a
lot more justification than none.
It was nested in v9, you complained, so that was removed in v10.
It came back in v11 and is in ever since without complains from you - so silent
agreement was assumed.
However, the justification for adding it back was in the v10 discussion, yet
probably it did not hit your and others mailboxes - as I find the corresponding
mail in my mailbox (circulated via the ML, not a local copy) yet it does not
appear in the archives [1].
Maybe mails are lost without noticing longer than we thought...
The argumentation was as quoted:
[from Andreas]
1. Up until now, when decoding a series of stand-alone WebP pictures,
the multiple decoder instances don't wait for each other (because the
WebP decoder had no update_thread_context callback). You added such a
callback and now you are calling it after the main picture has already
been decoded, effectively serializing everything. You can test this for
yourself: Create lots of files via ffmpeg -i <input> -c:v libwebp -f
webp%d.webp and decode them (don't use -stream_loop on a single input
picture, as this will flush the decoder after every single picture, so
that everything is always serialized).
2. To fix this, ff_thread_finish_setup() needs to be called as soon as
possible. This means that you have to abandon the approach of letting
the inner VP8 decoder set the frame dimensions and then overwriting them
again in the WebP decoder.
so I tried to move the ff_thread_finish_setup() call and avoid updating the
AVCodecContext after decoding.
With the integrated decoder like here, ff_vp8_decode_frame() writes into the
AVCodecContext while decoding, so ff_thread_finish_setup() can only be called
afterwards -> quasi sequentially. I do see decoding speed for many files at
once dropping from 100x to 16x compared to master.
With the decoder decoupled, I can actually move ff_thread_finish_setup() before
send_packet() (in contrast to v9) -> next thread can start before decoding the
frame.
I don't see a penalty with that way on decoding many files at once compared to
master, as expected.
Is that reason enough to actually decouple the vp8 decoder?
Do you think it is reason enough? Or you still want this to use the ff_
internal API?
-Thilo
[1] https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2024-February/thread.html
_______________________________________________
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".
