diff mbox series

[FFmpeg-devel,v13,2/8] avcodec/webp: separate VP8 decoding

Message ID 20240621104323.92453-3-thilo.borgmann@mail.de
State New
Headers show
Series webp: add support for animated WebP decoding | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Thilo Borgmann June 21, 2024, 10:43 a.m. UTC 
From: Thilo Borgmann via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>

---
 libavcodec/webp.c | 50 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 6 deletions(-)

Comments

Anton Khirnov June 21, 2024, 11:52 a.m. UTC | #1 
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.

And even if a nested decoder is unavoidable for some reason, it need a
lot more justification than none.
diff mbox series

Patch

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 */
     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 */
@@ -1299,12 +1300,13 @@  static int vp8_lossy_decode_frame(AVCodecContext *avctx, AVFrame *p,
     int ret;
 
     if (!s->initialized) {
-        ff_vp8_decode_init(avctx);
+        VP8Context *s_vp8 = s->avctx_vp8->priv_data;
+        s_vp8->actually_webp = 1;
         s->initialized = 1;
-        s->v.actually_webp = 1;
     }
     avctx->pix_fmt = s->has_alpha ? AV_PIX_FMT_YUVA420P : AV_PIX_FMT_YUV420P;
     s->lossless = 0;
+    s->avctx_vp8->pix_fmt = avctx->pix_fmt;
 
     if (data_size > INT_MAX) {
         av_log(avctx, AV_LOG_ERROR, "unsupported chunk size\n");
@@ -1315,14 +1317,32 @@  static int vp8_lossy_decode_frame(AVCodecContext *avctx, AVFrame *p,
     s->pkt->data = data_start;
     s->pkt->size = data_size;
 
-    ret = ff_vp8_decode_frame(avctx, p, got_frame, s->pkt);
-    if (ret < 0)
+    ret = avcodec_send_packet(s->avctx_vp8, s->pkt);
+    if (ret < 0) {
+        av_log(avctx, AV_LOG_ERROR, "Error submitting a packet for decoding\n");
         return ret;
+    }
 
-    if (!*got_frame)
+    ret = avcodec_receive_frame(s->avctx_vp8, p);
+    if (ret < 0) {
+        av_log(avctx, AV_LOG_ERROR, "VP8 decoding error: %s.\n", av_err2str(ret));
         return AVERROR_INVALIDDATA;
+    }
+
+    ret = ff_decode_frame_props(avctx, p);
+    if (ret < 0) {
+        return ret;
+    }
+
+    if (!p->private_ref) {
+        ret = ff_attach_decode_data(p);
+        if (ret < 0) {
+            return ret;
+        }
+    }
 
-    update_canvas_size(avctx, avctx->width, avctx->height);
+    *got_frame = 1;
+    update_canvas_size(avctx, s->avctx_vp8->width, s->avctx_vp8->height);
 
     if (s->has_alpha) {
         ret = vp8_lossy_decode_alpha(avctx, p, s->alpha_data,
@@ -1539,11 +1559,28 @@  exif_end:
 static av_cold int webp_decode_init(AVCodecContext *avctx)
 {
     WebPContext *s = avctx->priv_data;
+    int ret;
+    const AVCodec *codec;
 
     s->pkt = av_packet_alloc();
     if (!s->pkt)
         return AVERROR(ENOMEM);
 
+
+    /* Prepare everything needed for VP8 decoding */
+    codec = avcodec_find_decoder(AV_CODEC_ID_VP8);
+    if (!codec)
+        return AVERROR_BUG;
+    s->avctx_vp8 = avcodec_alloc_context3(codec);
+    if (!s->avctx_vp8)
+        return AVERROR(ENOMEM);
+    s->avctx_vp8->flags = avctx->flags;
+    s->avctx_vp8->flags2 = avctx->flags2;
+    s->avctx_vp8->pix_fmt = avctx->pix_fmt;
+    ret = avcodec_open2(s->avctx_vp8, codec, NULL);
+    if (ret < 0) {
+        return ret;
+    }
     return 0;
 }
 
@@ -1552,6 +1589,7 @@  static av_cold int webp_decode_close(AVCodecContext *avctx)
     WebPContext *s = avctx->priv_data;
 
     av_packet_free(&s->pkt);
+    avcodec_free_context(&s->avctx_vp8);
 
     if (s->initialized)
         return ff_vp8_decode_free(avctx);