[FFmpeg-devel] [PATCH 1/2] libavcodec: add support for animated WebP decoding

[Zlomek, Josef]

On Wed, Jul 8, 2020 at 11:57 AM Lynne <dev at lynne.ee> wrote:

> Jul 8, 2020, 06:28 by josef at pex.com:
>
> > Fixes: 4907
> >
> > Adds support for decoding of animated WebP.
> >
> > The WebP parser now splits the input stream into packets containing one
> frame.
> >
> > 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.
> >
>
> We don't convert pixel formats in decoders, and I wouldn't want to have
> libavcodec
> depend on libswscale. I wouldn't trust libswscale to make accurate
> conversions either.
>
> Can you use the macros in libavutil/colorspace.h to convert the BGRA value
> to YUVA
> and then just memcpy it across the frame?
>

Thank you for the tip!
I could not find out how to do it better than I did.

Also, there are a lot of frame memcpys in the code. Could you get rid of
> most of them
> by refcounting?
>

The memcpy is needed. The frames of the WebP animation do not cover the
whole canvas of the picture,
i.e. the decoded VP8 frame is just a sub-rectangle of the canvas. The frame
changes just a part of the canvas.

The frame needs to be copied to the proper position in the canvas, or
merged into the canvas using alpha blending.
The canvas is then copied to the output frame, and we need to keep it
because it will be partially overwritten by subsequent frames.


> -    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,> +
>   .capabilities   = AV_CODEC_CAP_DR1,
> Why?
>

As described before, the subsequent frame depends on the previous ones.
With threading enabled, only some frames are decoded and many errors are
printed.
I will experiment with marking partial frames as not key frames, maybe it
will fix the problems with threading.

> +            if (component == 1 || component == 2) {> +
> height = AV_CEIL_RSHIFT(height, desc->log2_chroma_h);> +            }
> We don't wrap 1-line if statements in brackets.
>

I will fix such occurrences in my code.

Thank you!

Josef