[FFmpeg-devel] [PATCH v2] avcodec: Factor updating palette out

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Mon Mar 22 00:43:47 EET 2021


Because the properties of frames returned from ff_get/reget_buffer
are not reset at all, lots of returned frames had palette_has_changed
wrongly set to 1. This has been changed, too.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
Moved it to decode.h as requested.

 libavcodec/8bps.c           | 12 ++----------
 libavcodec/cinepak.c        | 10 ++--------
 libavcodec/decode.c         | 14 ++++++++++++++
 libavcodec/decode.h         |  9 +++++++++
 libavcodec/gdv.c            |  6 ++----
 libavcodec/idcinvideo.c     | 10 ++--------
 libavcodec/imx.c            |  6 ++----
 libavcodec/interplayvideo.c | 10 ++--------
 libavcodec/kmvc.c           |  9 ++-------
 libavcodec/msrle.c          | 12 +++---------
 libavcodec/msvideo1.c       | 11 ++---------
 libavcodec/qpeg.c           | 10 ++--------
 libavcodec/qtrle.c          | 10 +---------
 libavcodec/rawdec.c         | 13 ++-----------
 libavcodec/rscc.c           | 14 +++-----------
 libavcodec/smc.c            | 10 ++--------
 libavcodec/tscc.c           | 11 ++---------
 17 files changed, 54 insertions(+), 123 deletions(-)

diff --git a/libavcodec/8bps.c b/libavcodec/8bps.c
index 53e939d35d..dda8b8b1c9 100644
--- a/libavcodec/8bps.c
+++ b/libavcodec/8bps.c
@@ -37,6 +37,7 @@
 #include "libavutil/internal.h"
 #include "libavutil/intreadwrite.h"
 #include "avcodec.h"
+#include "decode.h"
 #include "internal.h"
 
 
@@ -122,16 +123,7 @@ static int decode_frame(AVCodecContext *avctx, void *data,
     }
 
     if (avctx->bits_per_coded_sample <= 8) {
-        buffer_size_t size;
-        const uint8_t *pal = av_packet_get_side_data(avpkt,
-                                                     AV_PKT_DATA_PALETTE,
-                                                     &size);
-        if (pal && size == AVPALETTE_SIZE) {
-            frame->palette_has_changed = 1;
-            memcpy(c->pal, pal, AVPALETTE_SIZE);
-        } else if (pal) {
-            av_log(avctx, AV_LOG_ERROR, "Palette size %d is wrong\n", size);
-        }
+        frame->palette_has_changed = ff_copy_palette(c->pal, avpkt, avctx);
 
         memcpy (frame->data[1], c->pal, AVPALETTE_SIZE);
     }
diff --git a/libavcodec/cinepak.c b/libavcodec/cinepak.c
index d70cb4b694..ccd6af3208 100644
--- a/libavcodec/cinepak.c
+++ b/libavcodec/cinepak.c
@@ -40,6 +40,7 @@
 #include "libavutil/common.h"
 #include "libavutil/intreadwrite.h"
 #include "avcodec.h"
+#include "decode.h"
 #include "internal.h"
 
 
@@ -477,14 +478,7 @@ static int cinepak_decode_frame(AVCodecContext *avctx,
         return ret;
 
     if (s->palette_video) {
-        buffer_size_t size;
-        const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &size);
-        if (pal && size == AVPALETTE_SIZE) {
-            s->frame->palette_has_changed = 1;
-            memcpy(s->pal, pal, AVPALETTE_SIZE);
-        } else if (pal) {
-            av_log(avctx, AV_LOG_ERROR, "Palette size %d is wrong\n", size);
-        }
+        s->frame->palette_has_changed = ff_copy_palette(s->pal, avpkt, avctx);
     }
 
     if ((ret = cinepak_decode(s)) < 0) {
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 390147908d..0956a6ac6f 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -2091,3 +2091,17 @@ FF_ENABLE_DEPRECATION_WARNINGS
 
     return 0;
 }
+
+int ff_copy_palette(void *dst, const AVPacket *src, void *logctx)
+{
+    buffer_size_t size;
+    const void *pal = av_packet_get_side_data(src, AV_PKT_DATA_PALETTE, &size);
+
+    if (pal && size == AVPALETTE_SIZE) {
+        memcpy(dst, pal, AVPALETTE_SIZE);
+        return 1;
+    } else if (pal) {
+        av_log(logctx, AV_LOG_ERROR, "Palette size %d is wrong\n", size);
+    }
+    return 0;
+}
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index 1467b1eb33..dee2543b1c 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -79,6 +79,15 @@ int ff_decode_get_hw_frames_ctx(AVCodecContext *avctx,
 
 int ff_attach_decode_data(AVFrame *frame);
 
+/**
+ * Check whether the side-data of src contains a palette of
+ * size AVPALETTE_SIZE; if so, copy it to dst and return 1;
+ * else return 0.
+ * Also emit an error message upon encountering a palette
+ * with invalid size.
+ */
+int ff_copy_palette(void *dst, const AVPacket *src, void *logctx);
+
 /**
  * Perform decoder initialization and validation.
  * Called when opening the decoder, before the AVCodec.init() call.
diff --git a/libavcodec/gdv.c b/libavcodec/gdv.c
index 860634c9ec..406d79df01 100644
--- a/libavcodec/gdv.c
+++ b/libavcodec/gdv.c
@@ -23,6 +23,7 @@
 #include "libavutil/common.h"
 #include "avcodec.h"
 #include "bytestream.h"
+#include "decode.h"
 #include "internal.h"
 
 typedef struct GDVContext {
@@ -462,8 +463,6 @@ static int gdv_decode_frame(AVCodecContext *avctx, void *data,
     PutByteContext *pb = &gdv->pb;
     AVFrame *frame = data;
     int ret, i;
-    buffer_size_t pal_size;
-    const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &pal_size);
     int compression;
     unsigned flags;
     uint8_t *dst;
@@ -479,8 +478,7 @@ static int gdv_decode_frame(AVCodecContext *avctx, void *data,
 
     if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
         return ret;
-    if (pal && pal_size == AVPALETTE_SIZE)
-        memcpy(gdv->pal, pal, AVPALETTE_SIZE);
+    ff_copy_palette(gdv->pal, avpkt, avctx);
 
     if (compression < 2 && bytestream2_get_bytes_left(gb) < 256*3)
         return AVERROR_INVALIDDATA;
diff --git a/libavcodec/idcinvideo.c b/libavcodec/idcinvideo.c
index 569191511f..b79ec93108 100644
--- a/libavcodec/idcinvideo.c
+++ b/libavcodec/idcinvideo.c
@@ -49,6 +49,7 @@
 #include <string.h>
 
 #include "avcodec.h"
+#include "decode.h"
 #include "internal.h"
 #include "libavutil/internal.h"
 
@@ -214,8 +215,6 @@ static int idcin_decode_frame(AVCodecContext *avctx,
     const uint8_t *buf = avpkt->data;
     int buf_size = avpkt->size;
     IdcinContext *s = avctx->priv_data;
-    buffer_size_t pal_size;
-    const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &pal_size);
     AVFrame *frame = data;
     int ret;
 
@@ -228,12 +227,7 @@ static int idcin_decode_frame(AVCodecContext *avctx,
     if (idcin_decode_vlcs(s, frame))
         return AVERROR_INVALIDDATA;
 
-    if (pal && pal_size == AVPALETTE_SIZE) {
-        frame->palette_has_changed = 1;
-        memcpy(s->pal, pal, AVPALETTE_SIZE);
-    } else if (pal) {
-        av_log(avctx, AV_LOG_ERROR, "Palette size %d is wrong\n", pal_size);
-    }
+    frame->palette_has_changed = ff_copy_palette(s->pal, avpkt, avctx);
     /* make the palette available on the way out */
     memcpy(frame->data[1], s->pal, AVPALETTE_SIZE);
 
diff --git a/libavcodec/imx.c b/libavcodec/imx.c
index 0d6c99e5bf..d86c747c6d 100644
--- a/libavcodec/imx.c
+++ b/libavcodec/imx.c
@@ -21,6 +21,7 @@
 #include "libavutil/common.h"
 #include "avcodec.h"
 #include "bytestream.h"
+#include "decode.h"
 #include "internal.h"
 
 typedef struct SimbiosisIMXContext {
@@ -50,16 +51,13 @@ static int imx_decode_frame(AVCodecContext *avctx, void *data,
 {
     SimbiosisIMXContext *imx = avctx->priv_data;
     int ret, x, y;
-    buffer_size_t pal_size;
-    const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &pal_size);
     AVFrame *frame = imx->frame;
     GetByteContext gb;
 
     if ((ret = ff_reget_buffer(avctx, frame, 0)) < 0)
         return ret;
 
-    if (pal && pal_size == AVPALETTE_SIZE) {
-        memcpy(imx->pal, pal, pal_size);
+    if (ff_copy_palette(imx->pal, avpkt, avctx)) {
         frame->palette_has_changed = 1;
         frame->key_frame = 1;
     } else {
diff --git a/libavcodec/interplayvideo.c b/libavcodec/interplayvideo.c
index 4d16fdf619..388fe2665e 100644
--- a/libavcodec/interplayvideo.c
+++ b/libavcodec/interplayvideo.c
@@ -43,6 +43,7 @@
 #define BITSTREAM_READER_LE
 #include "avcodec.h"
 #include "bytestream.h"
+#include "decode.h"
 #include "get_bits.h"
 #include "hpeldsp.h"
 #include "internal.h"
@@ -1317,14 +1318,7 @@ static int ipvideo_decode_frame(AVCodecContext *avctx,
         return ret;
 
     if (!s->is_16bpp) {
-        buffer_size_t size;
-        const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &size);
-        if (pal && size == AVPALETTE_SIZE) {
-            frame->palette_has_changed = 1;
-            memcpy(s->pal, pal, AVPALETTE_SIZE);
-        } else if (pal) {
-            av_log(avctx, AV_LOG_ERROR, "Palette size %d is wrong\n", size);
-        }
+        frame->palette_has_changed = ff_copy_palette(s->pal, avpkt, avctx);
     }
 
     switch (frame_format) {
diff --git a/libavcodec/kmvc.c b/libavcodec/kmvc.c
index 0d96139220..cd9e32f443 100644
--- a/libavcodec/kmvc.c
+++ b/libavcodec/kmvc.c
@@ -29,6 +29,7 @@
 
 #include "avcodec.h"
 #include "bytestream.h"
+#include "decode.h"
 #include "internal.h"
 #include "libavutil/common.h"
 
@@ -268,8 +269,6 @@ static int decode_frame(AVCodecContext * avctx, void *data, int *got_frame,
     int i, ret;
     int header;
     int blocksize;
-    buffer_size_t pal_size;
-    const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &pal_size);
 
     bytestream2_init(&ctx->g, avpkt->data, avpkt->size);
 
@@ -304,12 +303,8 @@ static int decode_frame(AVCodecContext * avctx, void *data, int *got_frame,
         }
     }
 
-    if (pal && pal_size == AVPALETTE_SIZE) {
+    if (ff_copy_palette(ctx->pal, avpkt, avctx))
         frame->palette_has_changed = 1;
-        memcpy(ctx->pal, pal, AVPALETTE_SIZE);
-    } else if (pal) {
-        av_log(avctx, AV_LOG_ERROR, "Palette size %d is wrong\n", pal_size);
-    }
 
     if (ctx->setpal) {
         ctx->setpal = 0;
diff --git a/libavcodec/msrle.c b/libavcodec/msrle.c
index feea2b60bb..42c8f48373 100644
--- a/libavcodec/msrle.c
+++ b/libavcodec/msrle.c
@@ -33,6 +33,7 @@
 #include <string.h>
 
 #include "avcodec.h"
+#include "decode.h"
 #include "internal.h"
 #include "msrledec.h"
 #include "libavutil/imgutils.h"
@@ -97,15 +98,8 @@ static int msrle_decode_frame(AVCodecContext *avctx,
         return ret;
 
     if (avctx->bits_per_coded_sample > 1 && avctx->bits_per_coded_sample <= 8) {
-        buffer_size_t size;
-        const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &size);
-
-        if (pal && size == AVPALETTE_SIZE) {
-            s->frame->palette_has_changed = 1;
-            memcpy(s->pal, pal, AVPALETTE_SIZE);
-        } else if (pal) {
-            av_log(avctx, AV_LOG_ERROR, "Palette size %d is wrong\n", size);
-        }
+        s->frame->palette_has_changed = ff_copy_palette(s->pal, avpkt, avctx);
+
         /* make the palette available */
         memcpy(s->frame->data[1], s->pal, AVPALETTE_SIZE);
     }
diff --git a/libavcodec/msvideo1.c b/libavcodec/msvideo1.c
index b4b5ea4666..cfd51cfd67 100644
--- a/libavcodec/msvideo1.c
+++ b/libavcodec/msvideo1.c
@@ -33,6 +33,7 @@
 #include "libavutil/internal.h"
 #include "libavutil/intreadwrite.h"
 #include "avcodec.h"
+#include "decode.h"
 #include "internal.h"
 
 #define PALETTE_COUNT 256
@@ -314,15 +315,7 @@ static int msvideo1_decode_frame(AVCodecContext *avctx,
         return ret;
 
     if (s->mode_8bit) {
-        buffer_size_t size;
-        const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &size);
-
-        if (pal && size == AVPALETTE_SIZE) {
-            memcpy(s->pal, pal, AVPALETTE_SIZE);
-            s->frame->palette_has_changed = 1;
-        } else if (pal) {
-            av_log(avctx, AV_LOG_ERROR, "Palette size %d is wrong\n", size);
-        }
+        s->frame->palette_has_changed = ff_copy_palette(s->pal, avpkt, avctx);
     }
 
     if (s->mode_8bit)
diff --git a/libavcodec/qpeg.c b/libavcodec/qpeg.c
index f4edc4b16f..2e66d3bf8a 100644
--- a/libavcodec/qpeg.c
+++ b/libavcodec/qpeg.c
@@ -26,6 +26,7 @@
 
 #include "avcodec.h"
 #include "bytestream.h"
+#include "decode.h"
 #include "internal.h"
 
 typedef struct QpegContext{
@@ -274,8 +275,6 @@ static int decode_frame(AVCodecContext *avctx,
     AVFrame * const ref = a->ref;
     uint8_t* outdata;
     int delta, intra, ret;
-    buffer_size_t pal_size;
-    const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &pal_size);
 
     if (avpkt->size < 0x86) {
         av_log(avctx, AV_LOG_ERROR, "Packet is too small\n");
@@ -300,12 +299,7 @@ static int decode_frame(AVCodecContext *avctx,
     }
 
     /* make the palette available on the way out */
-    if (pal && pal_size == AVPALETTE_SIZE) {
-        p->palette_has_changed = 1;
-        memcpy(a->pal, pal, AVPALETTE_SIZE);
-    } else if (pal) {
-        av_log(avctx, AV_LOG_ERROR, "Palette size %d is wrong\n", pal_size);
-    }
+    p->palette_has_changed = ff_copy_palette(a->pal, avpkt, avctx);
     memcpy(p->data[1], a->pal, AVPALETTE_SIZE);
 
     av_frame_unref(ref);
diff --git a/libavcodec/qtrle.c b/libavcodec/qtrle.c
index e04fd31431..33da393bd1 100644
--- a/libavcodec/qtrle.c
+++ b/libavcodec/qtrle.c
@@ -540,15 +540,7 @@ static int qtrle_decode_frame(AVCodecContext *avctx,
     }
 
     if(has_palette) {
-        buffer_size_t size;
-        const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &size);
-
-        if (pal && size == AVPALETTE_SIZE) {
-            s->frame->palette_has_changed = 1;
-            memcpy(s->pal, pal, AVPALETTE_SIZE);
-        } else if (pal) {
-            av_log(avctx, AV_LOG_ERROR, "Palette size %d is wrong\n", size);
-        }
+        s->frame->palette_has_changed = ff_copy_palette(s->pal, avpkt, avctx);
 
         /* make the palette available on the way out */
         memcpy(s->frame->data[1], s->pal, AVPALETTE_SIZE);
diff --git a/libavcodec/rawdec.c b/libavcodec/rawdec.c
index c18c3dd3e1..1badb69180 100644
--- a/libavcodec/rawdec.c
+++ b/libavcodec/rawdec.c
@@ -367,16 +367,8 @@ static int raw_decode(AVCodecContext *avctx, void *data, int *got_frame,
     }
 
     if (avctx->pix_fmt == AV_PIX_FMT_PAL8) {
-        buffer_size_t pal_size;
-        const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE,
-                                                     &pal_size);
         int ret;
 
-        if (pal && pal_size != AVPALETTE_SIZE) {
-            av_log(avctx, AV_LOG_ERROR, "Palette size %d is wrong\n", pal_size);
-            pal = NULL;
-        }
-
         if (!context->palette)
             context->palette = av_buffer_alloc(AVPALETTE_SIZE);
         if (!context->palette) {
@@ -389,15 +381,14 @@ static int raw_decode(AVCodecContext *avctx, void *data, int *got_frame,
             return ret;
         }
 
-        if (pal) {
-            memcpy(context->palette->data, pal, AVPALETTE_SIZE);
+        if (ff_copy_palette(context->palette->data, avpkt, avctx)) {
             frame->palette_has_changed = 1;
         } else if (context->is_nut_pal8) {
             int vid_size = avctx->width * avctx->height;
             int pal_size = avpkt->size - vid_size;
 
             if (avpkt->size > vid_size && pal_size <= AVPALETTE_SIZE) {
-                pal = avpkt->data + vid_size;
+                const uint8_t *pal = avpkt->data + vid_size;
                 memcpy(context->palette->data, pal, pal_size);
                 frame->palette_has_changed = 1;
             }
diff --git a/libavcodec/rscc.c b/libavcodec/rscc.c
index 79b02da441..7b27127bf5 100644
--- a/libavcodec/rscc.c
+++ b/libavcodec/rscc.c
@@ -43,6 +43,7 @@
 
 #include "avcodec.h"
 #include "bytestream.h"
+#include "decode.h"
 #include "internal.h"
 
 #define TILE_SIZE 8
@@ -346,17 +347,8 @@ static int rscc_decode_frame(AVCodecContext *avctx, void *data,
 
     /* Palette handling */
     if (avctx->pix_fmt == AV_PIX_FMT_PAL8) {
-        buffer_size_t size;
-        const uint8_t *palette = av_packet_get_side_data(avpkt,
-                                                         AV_PKT_DATA_PALETTE,
-                                                         &size);
-        if (palette && size == AVPALETTE_SIZE) {
-            frame->palette_has_changed = 1;
-            memcpy(ctx->palette, palette, AVPALETTE_SIZE);
-        } else if (palette) {
-            av_log(avctx, AV_LOG_ERROR, "Palette size %d is wrong\n", size);
-        }
-        memcpy (frame->data[1], ctx->palette, AVPALETTE_SIZE);
+        frame->palette_has_changed = ff_copy_palette(ctx->palette, avpkt, avctx);
+        memcpy(frame->data[1], ctx->palette, AVPALETTE_SIZE);
     }
     // We only return a picture when enough of it is undamaged, this avoids copying nearly broken frames around
     if (ctx->valid_pixels < ctx->inflated_size)
diff --git a/libavcodec/smc.c b/libavcodec/smc.c
index 33581bbf81..880bf0620e 100644
--- a/libavcodec/smc.c
+++ b/libavcodec/smc.c
@@ -35,6 +35,7 @@
 #include "libavutil/intreadwrite.h"
 #include "avcodec.h"
 #include "bytestream.h"
+#include "decode.h"
 #include "internal.h"
 
 #define CPAIR 2
@@ -435,8 +436,6 @@ static int smc_decode_frame(AVCodecContext *avctx,
     const uint8_t *buf = avpkt->data;
     int buf_size = avpkt->size;
     SmcContext *s = avctx->priv_data;
-    buffer_size_t pal_size;
-    const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &pal_size);
     int ret;
     int total_blocks = ((s->avctx->width + 3) / 4) * ((s->avctx->height + 3) / 4);
 
@@ -448,12 +447,7 @@ static int smc_decode_frame(AVCodecContext *avctx,
     if ((ret = ff_reget_buffer(avctx, s->frame, 0)) < 0)
         return ret;
 
-    if (pal && pal_size == AVPALETTE_SIZE) {
-        s->frame->palette_has_changed = 1;
-        memcpy(s->pal, pal, AVPALETTE_SIZE);
-    } else if (pal) {
-        av_log(avctx, AV_LOG_ERROR, "Palette size %d is wrong\n", pal_size);
-    }
+    s->frame->palette_has_changed = ff_copy_palette(s->pal, avpkt, avctx);
 
     smc_decode_stream(s);
 
diff --git a/libavcodec/tscc.c b/libavcodec/tscc.c
index d33639f3a4..f67a910c73 100644
--- a/libavcodec/tscc.c
+++ b/libavcodec/tscc.c
@@ -38,6 +38,7 @@
 #include <stdlib.h>
 
 #include "avcodec.h"
+#include "decode.h"
 #include "internal.h"
 #include "msrledec.h"
 
@@ -72,15 +73,7 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
     int palette_has_changed = 0;
 
     if (c->avctx->pix_fmt == AV_PIX_FMT_PAL8) {
-        buffer_size_t size;
-        const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &size);
-
-        if (pal && size == AVPALETTE_SIZE) {
-            palette_has_changed = 1;
-            memcpy(c->pal, pal, AVPALETTE_SIZE);
-        } else if (pal) {
-            av_log(avctx, AV_LOG_ERROR, "Palette size %d is wrong\n", size);
-        }
+        palette_has_changed = ff_copy_palette(c->pal, avpkt, avctx);
     }
 
     ret = inflateReset(&c->zstream);
-- 
2.27.0



More information about the ffmpeg-devel mailing list