Re: [FFmpeg-devel] [PATCH v10 2/5] libavcodec/webp: add support for animated WebP

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

Hi,

Am 19.02.24 um 17:50 schrieb Andreas Rheinhardt:
> Thilo Borgmann via ffmpeg-devel:
>> From: Josef Zlomek <josef@pex.com>
>>
>> Fixes: 4907
>>
>> Adds support for decoding of animated WebP.
>>
>> The WebP decoder adds the animation related features according to the specs:
>> https://developers.google.com/speed/webp/docs/riff_container#animation
>> The frames of the animation may be smaller than the image canvas.
>> Therefore, the frame is decoded to a temporary frame,
>> then it is blended into the canvas, the canvas is copied to the output frame,
>> and finally the frame is disposed from the canvas.
>>
>> The output to AV_PIX_FMT_YUVA420P/AV_PIX_FMT_YUV420P is still supported.
>> The background color is specified only as BGRA in the WebP file
>> so it is converted to YUVA if YUV formats are output.
>>
>> Signed-off-by: Josef Zlomek <josef@pex.com>
>> ---
>>   Changelog               |   1 +
>>   libavcodec/codec_desc.c |   3 +-
>>   libavcodec/version.h    |   2 +-
>>   libavcodec/webp.c       | 704 +++++++++++++++++++++++++++++++++++++---
>>   4 files changed, 654 insertions(+), 56 deletions(-)
>>
> 
>> +static int webp_decode_frame(AVCodecContext *avctx, AVFrame *p,
>> +                             int *got_frame, AVPacket *avpkt)
>> +{
>> +    WebPContext *s = avctx->priv_data;
>> +    AVFrame *canvas = s->canvas_frame.f;
>> +    int ret;
>> +    int key_frame = avpkt->flags & AV_PKT_FLAG_KEY;
>> +
>> +    *got_frame   = 0;
>> +
>> +    if (key_frame) {
>> +        // The canvas is passed from one thread to another in a sequence
>> +        // starting with a key frame followed by non-key frames.
>> +        // The key frame reports progress 1,
>> +        // the N-th non-key frame awaits progress N = s->await_progress
>> +        // and reports progress N + 1.
>> +        s->await_progress = 0;
>> +    }
>> +
>> +    // reset the frame params
>> +    s->anmf_flags = 0;
>> +    s->width      = 0;
>> +    s->height     = 0;
>> +    s->pos_x      = 0;
>> +    s->pos_y      = 0;
>> +    s->has_alpha  = 0;
>> +
>> +    ret = webp_decode_frame_common(avctx, avpkt->data, avpkt->size, got_frame, key_frame);
>> +    if (ret < 0)
>> +        goto end;
>> +
>> +    if (s->vp8x_flags & VP8X_FLAG_ANIMATION) {
>> +        // VP8 decoder might have changed the width and height of the frame
>> +        AVFrame *frame  = s->frame;
>> +        ret = av_frame_copy_props(canvas, frame);
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        ret = ff_set_dimensions(s->avctx, canvas->width, canvas->height);
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        s->avctx->pix_fmt = canvas->format;
>> +    }
>> +
>> +    ff_thread_finish_setup(s->avctx);
> 

> 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?


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".