diff mbox series

[FFmpeg-devel,1/2,v2] avcodec/gifenc: Add global_palette option

Message ID 20210220185050.508373-2-derek.buitenhuis@gmail.com
State Accepted
Commit 5f2804aba71dbf4833a3cc5e7b30e8351cd9037a
Headers show
Series GIF Palette Improvements
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Derek Buitenhuis Feb. 20, 2021, 6:50 p.m. UTC
This option will disable the writing of the global palette in global
GIF header if it is set to 0, causing only the frame-level palette
to ever be written.

This will be useful later on when further frame-level palette
optimizations are introduced.

The default is 1, which maintains current default behavior.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
Does this need a avcodec minor/patch version bump?
---
 libavcodec/gif.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

Comments

Marton Balint Feb. 20, 2021, 7:58 p.m. UTC | #1
On Sat, 20 Feb 2021, Derek Buitenhuis wrote:

> This option will disable the writing of the global palette in global
> GIF header if it is set to 0, causing only the frame-level palette
> to ever be written.
>
> This will be useful later on when further frame-level palette
> optimizations are introduced.
>
> The default is 1, which maintains current default behavior.
>
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
> Does this need a avcodec minor/patch version bump?
> ---
> libavcodec/gif.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)

docs update is missing for the new option.

Regards,
Marton

>
> diff --git a/libavcodec/gif.c b/libavcodec/gif.c
> index de41992851..8c07ee2769 100644
> --- a/libavcodec/gif.c
> +++ b/libavcodec/gif.c
> @@ -51,6 +51,7 @@ typedef struct GIFContext {
>     AVFrame *last_frame;
>     int flags;
>     int image;
> +    int use_global_palette;
>     uint32_t palette[AVPALETTE_COUNT];  ///< local reference palette for !pal8
>     int palette_loaded;
>     int transparent_index;
> @@ -293,12 +294,14 @@ static int gif_image_write_image(AVCodecContext *avctx,
>
>         bcid = get_palette_transparency_index(global_palette);
> 
> -        bytestream_put_byte(bytestream, 0xf7); /* flags: global clut, 256 entries */
> +        bytestream_put_byte(bytestream, ((uint8_t) s->use_global_palette << 7) | 0x70 | (s->use_global_palette ? 7 : 0)); /* flags: global clut, 256 entries */
>         bytestream_put_byte(bytestream, bcid < 0 ? DEFAULT_TRANSPARENCY_INDEX : bcid); /* background color index */
>         bytestream_put_byte(bytestream, aspect);
> -        for (int i = 0; i < 256; i++) {
> -            const uint32_t v = global_palette[i] & 0xffffff;
> -            bytestream_put_be24(bytestream, v);
> +        if (s->use_global_palette) {
> +            for (int i = 0; i < 256; i++) {
> +                const uint32_t v = global_palette[i] & 0xffffff;
> +                bytestream_put_be24(bytestream, v);
> +            }
>         }
>     }
> 
> @@ -330,15 +333,16 @@ static int gif_image_write_image(AVCodecContext *avctx,
>     bytestream_put_le16(bytestream, width);
>     bytestream_put_le16(bytestream, height);
> 
> -    if (!palette) {
> -        bytestream_put_byte(bytestream, 0x00); /* flags */
> -    } else {
> +    if (palette || !s->use_global_palette) {
> +        const uint32_t *pal = palette ? palette : s->palette;
>         unsigned i;
>         bytestream_put_byte(bytestream, 1<<7 | 0x7); /* flags */
>         for (i = 0; i < AVPALETTE_COUNT; i++) {
> -            const uint32_t v = palette[i];
> +            const uint32_t v = pal[i];
>             bytestream_put_be24(bytestream, v);
>         }
> +    } else {
> +        bytestream_put_byte(bytestream, 0x00); /* flags */
>     }
>
>     bytestream_put_byte(bytestream, 0x08);
> @@ -473,6 +477,7 @@ static const AVOption gif_options[] = {
>         { "offsetting", "enable picture offsetting", 0, AV_OPT_TYPE_CONST, {.i64=GF_OFFSETTING}, INT_MIN, INT_MAX, FLAGS, "flags" },
>         { "transdiff", "enable transparency detection between frames", 0, AV_OPT_TYPE_CONST, {.i64=GF_TRANSDIFF}, INT_MIN, INT_MAX, FLAGS, "flags" },
>     { "gifimage", "enable encoding only images per frame", OFFSET(image), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS },
> +    { "global_palette", "write a palette to the global gif header where feasible", OFFSET(use_global_palette), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, FLAGS },
>     { NULL }
> };
> 
> -- 
> 2.30.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Derek Buitenhuis Feb. 21, 2021, 1:14 p.m. UTC | #2
On 20/02/2021 19:58, Marton Balint wrote:
> docs update is missing for the new option.

Seems there was no documentation at all, so I've add all of it,
and sent patch 3/2.

- Derek
Paul B Mahol Feb. 21, 2021, 6:56 p.m. UTC | #3
lgtm
diff mbox series

Patch

diff --git a/libavcodec/gif.c b/libavcodec/gif.c
index de41992851..8c07ee2769 100644
--- a/libavcodec/gif.c
+++ b/libavcodec/gif.c
@@ -51,6 +51,7 @@  typedef struct GIFContext {
     AVFrame *last_frame;
     int flags;
     int image;
+    int use_global_palette;
     uint32_t palette[AVPALETTE_COUNT];  ///< local reference palette for !pal8
     int palette_loaded;
     int transparent_index;
@@ -293,12 +294,14 @@  static int gif_image_write_image(AVCodecContext *avctx,
 
         bcid = get_palette_transparency_index(global_palette);
 
-        bytestream_put_byte(bytestream, 0xf7); /* flags: global clut, 256 entries */
+        bytestream_put_byte(bytestream, ((uint8_t) s->use_global_palette << 7) | 0x70 | (s->use_global_palette ? 7 : 0)); /* flags: global clut, 256 entries */
         bytestream_put_byte(bytestream, bcid < 0 ? DEFAULT_TRANSPARENCY_INDEX : bcid); /* background color index */
         bytestream_put_byte(bytestream, aspect);
-        for (int i = 0; i < 256; i++) {
-            const uint32_t v = global_palette[i] & 0xffffff;
-            bytestream_put_be24(bytestream, v);
+        if (s->use_global_palette) {
+            for (int i = 0; i < 256; i++) {
+                const uint32_t v = global_palette[i] & 0xffffff;
+                bytestream_put_be24(bytestream, v);
+            }
         }
     }
 
@@ -330,15 +333,16 @@  static int gif_image_write_image(AVCodecContext *avctx,
     bytestream_put_le16(bytestream, width);
     bytestream_put_le16(bytestream, height);
 
-    if (!palette) {
-        bytestream_put_byte(bytestream, 0x00); /* flags */
-    } else {
+    if (palette || !s->use_global_palette) {
+        const uint32_t *pal = palette ? palette : s->palette;
         unsigned i;
         bytestream_put_byte(bytestream, 1<<7 | 0x7); /* flags */
         for (i = 0; i < AVPALETTE_COUNT; i++) {
-            const uint32_t v = palette[i];
+            const uint32_t v = pal[i];
             bytestream_put_be24(bytestream, v);
         }
+    } else {
+        bytestream_put_byte(bytestream, 0x00); /* flags */
     }
 
     bytestream_put_byte(bytestream, 0x08);
@@ -473,6 +477,7 @@  static const AVOption gif_options[] = {
         { "offsetting", "enable picture offsetting", 0, AV_OPT_TYPE_CONST, {.i64=GF_OFFSETTING}, INT_MIN, INT_MAX, FLAGS, "flags" },
         { "transdiff", "enable transparency detection between frames", 0, AV_OPT_TYPE_CONST, {.i64=GF_TRANSDIFF}, INT_MIN, INT_MAX, FLAGS, "flags" },
     { "gifimage", "enable encoding only images per frame", OFFSET(image), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS },
+    { "global_palette", "write a palette to the global gif header where feasible", OFFSET(use_global_palette), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, FLAGS },
     { NULL }
 };