Am 14.06.23 um 11:42 schrieb Andreas Rheinhardt:
Thilo Borgmann:
---
  libavcodec/webp.c | 143 +++++++++++++++++++++++-----------------------
  1 file changed, 71 insertions(+), 72 deletions(-)

diff --git a/libavcodec/webp.c b/libavcodec/webp.c
index bee43fcf19..d3e3f85dd3 100644
--- a/libavcodec/webp.c
+++ b/libavcodec/webp.c
@@ -1337,7 +1337,77 @@ static int vp8_lossy_decode_frame(AVCodecContext *avctx, 
AVFrame *p,
      }
      return ret;
  }
-int init_canvas_frame(WebPContext *s, int format, int key_frame);
+
+static int init_canvas_frame(WebPContext *s, int format, int key_frame)
+{
+    AVFrame *canvas = s->canvas_frame.f;
+    int height;
+    int ret;
+
+    // canvas is needed only for animation
+    if (!(s->vp8x_flags & VP8X_FLAG_ANIMATION))
+        return 0;
+
+    // avoid init for non-key frames whose format and size did not change
+    if (!key_frame &&
+        canvas->data[0] &&
+        canvas->format == format &&
+        canvas->width  == s->canvas_width &&
+        canvas->height == s->canvas_height)
+        return 0;
+
+    // canvas changes within IPPP sequences will loose thread sync
+    // because of the ThreadFrame reallocation and will wait forever
+    // so if frame-threading is used, forbid canvas changes and unlock
+    // previous frames
+    if (!key_frame && canvas->data[0]) {
+        if (s->avctx->thread_count > 1) {
+            av_log(s->avctx, AV_LOG_WARNING, "Canvas change detected. The output 
will be damaged. Use -threads 1 to try decoding with best effort.\n");
+            // unlock previous frames that have sent an _await() call
+            ff_thread_report_progress(&s->canvas_frame, INT_MAX, 0);
+            return AVERROR_PATCHWELCOME;
+        } else {
+            // warn for damaged frames
+            av_log(s->avctx, AV_LOG_WARNING, "Canvas change detected. The output 
will be damaged.\n");
+        }
+    }
+
+    s->avctx->pix_fmt = format;
+    canvas->format    = format;
+    canvas->width     = s->canvas_width;
+    canvas->height    = s->canvas_height;
+
+    // VP8 decoder changed the width and height in AVCodecContext.
+    // Change it back to the canvas size.
+    ret = ff_set_dimensions(s->avctx, s->canvas_width, s->canvas_height);
+    if (ret < 0)
+        return ret;
+
+    ff_thread_release_ext_buffer(s->avctx, &s->canvas_frame);
+    ret = ff_thread_get_ext_buffer(s->avctx, &s->canvas_frame, 
AV_GET_BUFFER_FLAG_REF);
+    if (ret < 0)
+        return ret;
+
+    if (canvas->format == AV_PIX_FMT_ARGB) {
+        height = canvas->height;
+        memset(canvas->data[0], 0, height * canvas->linesize[0]);
+    } else /* if (canvas->format == AV_PIX_FMT_YUVA420P) */ {
+        const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(canvas->format);
+        for (int comp = 0; comp < desc->nb_components; comp++) {
+            int plane = desc->comp[comp].plane;
+
+            if (comp == 1 || comp == 2)
+                height = AV_CEIL_RSHIFT(canvas->height, desc->log2_chroma_h);
+            else
+                height = FFALIGN(canvas->height, 1 << desc->log2_chroma_h);
+
+            memset(canvas->data[plane], s->transparent_yuva[plane],
+                   height * canvas->linesize[plane]);
+        }
+    }
+
+    return 0;
+}
static int webp_decode_frame_common(AVCodecContext *avctx, uint8_t *data, int size,
                                 int *got_frame, int key_frame)
@@ -1577,77 +1647,6 @@ exif_end:
      return size;
  }
-int init_canvas_frame(WebPContext *s, int format, int key_frame)
-{
-    AVFrame *canvas = s->canvas_frame.f;
-    int height;
-    int ret;
-
-    // canvas is needed only for animation
-    if (!(s->vp8x_flags & VP8X_FLAG_ANIMATION))
-        return 0;
-
-    // avoid init for non-key frames whose format and size did not change
-    if (!key_frame &&
-        canvas->data[0] &&
-        canvas->format == format &&
-        canvas->width  == s->canvas_width &&
-        canvas->height == s->canvas_height)
-        return 0;
-
-    // canvas changes within IPPP sequences will loose thread sync
-    // because of the ThreadFrame reallocation and will wait forever
-    // so if frame-threading is used, forbid canvas changes and unlock
-    // previous frames
-    if (!key_frame && canvas->data[0]) {
-        if (s->avctx->thread_count > 1) {
-            av_log(s->avctx, AV_LOG_WARNING, "Canvas change detected. The output 
will be damaged. Use -threads 1 to try decoding with best effort.\n");
-            // unlock previous frames that have sent an _await() call
-            ff_thread_report_progress(&s->canvas_frame, INT_MAX, 0);
-            return AVERROR_PATCHWELCOME;
-        } else {
-            // warn for damaged frames
-            av_log(s->avctx, AV_LOG_WARNING, "Canvas change detected. The output 
will be damaged.\n");
-        }
-    }
-
-    s->avctx->pix_fmt = format;
-    canvas->format    = format;
-    canvas->width     = s->canvas_width;
-    canvas->height    = s->canvas_height;
-
-    // VP8 decoder changed the width and height in AVCodecContext.
-    // Change it back to the canvas size.
-    ret = ff_set_dimensions(s->avctx, s->canvas_width, s->canvas_height);
-    if (ret < 0)
-        return ret;
-
-    ff_thread_release_ext_buffer(s->avctx, &s->canvas_frame);
-    ret = ff_thread_get_ext_buffer(s->avctx, &s->canvas_frame, 
AV_GET_BUFFER_FLAG_REF);
-    if (ret < 0)
-        return ret;
-
-    if (canvas->format == AV_PIX_FMT_ARGB) {
-        height = canvas->height;
-        memset(canvas->data[0], 0, height * canvas->linesize[0]);
-    } else /* if (canvas->format == AV_PIX_FMT_YUVA420P) */ {
-        const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(canvas->format);
-        for (int comp = 0; comp < desc->nb_components; comp++) {
-            int plane = desc->comp[comp].plane;
-
-            if (comp == 1 || comp == 2)
-                height = AV_CEIL_RSHIFT(canvas->height, desc->log2_chroma_h);
-            else
-                height = FFALIGN(canvas->height, 1 << desc->log2_chroma_h);
-
-            memset(canvas->data[plane], s->transparent_yuva[plane],
-                   height * canvas->linesize[plane]);
-        }
-    }
-
-    return 0;
-}
-
  /*
   * Blend src1 (foreground) and src2 (background) into dest, in ARGB format.
   * width, height are the dimensions of src1

You add this function in the preceding patch. So why don't you do it
properly instead of fixing this up later?

Done for Anton's remark in the last iteration to make the patch reviewable.

webp_decode_frame() was renamed into webp_decode_frame_common() which makes patch 3/4 very unreadable as the - lines do not correspond to the + lines, making it very hard/impossible to review.

Moving init_canvas_frame() out of the way into another place in the file resolves this mangling up of the patch so that the changes in webp:decode_frame_common() become readable again.

I could totally stash patch 4/4 into 3/4 before pushing if you want the history less readable but cleaner.

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-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to