[FFmpeg-devel] [PATCH v3 1/4] avcodec/webp: compatibilize with avformat/webpdec
Martin Reboredo
yakoyoku at gmail.com
Sun Sep 12 23:50:37 EEST 2021
Andreas Rheinhardt:
> Martin Reboredo:
> > The demuxer implementation splits some RIFF chunks (`RIFF`/`VP8X`/`ANMF` + frame chunk) or sends the picture chunks separately.
> > The internal WebP decoder waits for a complete file instead and by consequence it needs to be modified to support this kind of fractioned input.
> >
> > Fixes FATE tests with WebP.
> >
> > Signed-off-by: Martin Reboredo <yakoyoku at gmail.com>
> > ---
> > libavcodec/webp.c | 41 ++++++++++++++++++++++++++++++++---------
> > 1 file changed, 32 insertions(+), 9 deletions(-)
> >
> > diff --git a/libavcodec/webp.c b/libavcodec/webp.c
> > index 3efd4438d9..7858d69481 100644
> > --- a/libavcodec/webp.c
> > +++ b/libavcodec/webp.c
> > @@ -40,6 +40,7 @@
> > * - XMP metadata
> > */
> >
> > +#include "libavformat/internal.h"
> > #include "libavutil/imgutils.h"
> >
> > #define BITSTREAM_READER_LE
> > @@ -191,6 +192,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 */
> > + int read_header; /* RIFF header has been read */
> > int initialized; /* set once the VP8 context is initialized */
> > int has_alpha; /* has a separate alpha chunk */
> > enum AlphaCompression alpha_compression; /* compression type for alpha chunk */
> > @@ -1353,17 +1355,36 @@ static int webp_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
> > if (bytestream2_get_bytes_left(&gb) < 12)
> > return AVERROR_INVALIDDATA;
> >
> > - if (bytestream2_get_le32(&gb) != MKTAG('R', 'I', 'F', 'F')) {
> > - av_log(avctx, AV_LOG_ERROR, "missing RIFF tag\n");
> > - return AVERROR_INVALIDDATA;
> > - }
> > -
> > + chunk_type = bytestream2_get_le32(&gb);
> > chunk_size = bytestream2_get_le32(&gb);
> > - if (bytestream2_get_bytes_left(&gb) < chunk_size)
> > - return AVERROR_INVALIDDATA;
> >
> > - if (bytestream2_get_le32(&gb) != MKTAG('W', 'E', 'B', 'P')) {
> > - av_log(avctx, AV_LOG_ERROR, "missing WEBP tag\n");
> > + for (int i = 0; !s->read_header && i < 4; i++) {
> > + ff_lock_avformat();
>
> 1. Why are you locking avformat? What is this lock supposed to guard?
> 2. You can't lock avformat from avcodec at all: The ff_-prefix means
> that this function is intra-library (in this case libavformat) only. It
> will work with static builds, but it won't work with shared builds.
The locking mechanism was meant for parsing the WebP header that is not a single image i.e. sending the animated packages. For simple files a situation like
this is not going to happen because the entire file was in the package, but for animations the parsing turns out to be difficult while using concurrency.
I could do a much better implementation, at the moment I don't have any better fixes for it.
As for the libavformat lock guard it was a _horrible_ production error from my side, I will submit the fix.
> > +
> > + if (s->read_header) {
> > + ff_unlock_avformat();
> > + break;
> > + }
> > +
> > + if (chunk_type == MKTAG('R', 'I', 'F', 'F')) {
> > + int left = bytestream2_get_bytes_left(&gb);
> > + if (left < chunk_size && left != 22) {
> > + ff_unlock_avformat();
> > + return AVERROR_INVALIDDATA;
More information about the ffmpeg-devel
mailing list