diff mbox series

[FFmpeg-devel,v3,1/4] avcodec/webp: compatibilize with avformat/webpdec

Message ID 20210912202010.1542872-2-yakoyoku@gmail.com
State New
Headers show
Series [FFmpeg-devel,v3,1/4] avcodec/webp: compatibilize with avformat/webpdec
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 fail Make fate failed
andriy/make_ppc success Make finished
andriy/make_fate_ppc fail Make fate failed

Commit Message

Martin Reboredo Sept. 12, 2021, 8:20 p.m. UTC
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@gmail.com>
---
 libavcodec/webp.c | 41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)

Comments

Andreas Rheinhardt Sept. 12, 2021, 8:24 p.m. UTC | #1
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@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.

> +
> +        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;
> +            }
> +            if (bytestream2_get_le32(&gb) != MKTAG('W', 'E', 'B', 'P')) {
> +                av_log(avctx, AV_LOG_ERROR, "missing WEBP tag\n");
> +                ff_unlock_avformat();
> +                return AVERROR_INVALIDDATA;
> +            }
> +            s->read_header = 1;
> +        }
> +
> +        ff_unlock_avformat();
> +    }
> +
> +    if (!s->read_header) {
> +        av_log(avctx, AV_LOG_ERROR, "missing RIFF tag\n");
>          return AVERROR_INVALIDDATA;
>      }
>  
> @@ -1416,6 +1437,8 @@ static int webp_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
>              ret = av_image_check_size(s->width, s->height, 0, avctx);
>              if (ret < 0)
>                  return ret;
> +            if (bytestream2_get_bytes_left(&gb) == 0)
> +                return AVERROR(EAGAIN);
>              break;
>          case MKTAG('A', 'L', 'P', 'H'): {
>              int alpha_header, filter_m, compression;
>
diff mbox series

Patch

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();
+
+        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;
+            }
+            if (bytestream2_get_le32(&gb) != MKTAG('W', 'E', 'B', 'P')) {
+                av_log(avctx, AV_LOG_ERROR, "missing WEBP tag\n");
+                ff_unlock_avformat();
+                return AVERROR_INVALIDDATA;
+            }
+            s->read_header = 1;
+        }
+
+        ff_unlock_avformat();
+    }
+
+    if (!s->read_header) {
+        av_log(avctx, AV_LOG_ERROR, "missing RIFF tag\n");
         return AVERROR_INVALIDDATA;
     }
 
@@ -1416,6 +1437,8 @@  static int webp_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
             ret = av_image_check_size(s->width, s->height, 0, avctx);
             if (ret < 0)
                 return ret;
+            if (bytestream2_get_bytes_left(&gb) == 0)
+                return AVERROR(EAGAIN);
             break;
         case MKTAG('A', 'L', 'P', 'H'): {
             int alpha_header, filter_m, compression;