diff mbox

[FFmpeg-devel] lavfi/palettegen: Allow setting the background colour

Message ID CAB0OVGqW40Ubt1HoFD3qXatYVhm=PQJ4j5aBB-Qg3y5=WLnRZA@mail.gmail.com
State Accepted
Headers show

Commit Message

Carl Eugen Hoyos Oct. 17, 2017, 9:42 p.m. UTC 
Hi!

Attached patch is useful in combination with the transparency patch
for paletteuse.

Please comment, Carl Eugen

Comments

Carl Eugen Hoyos Oct. 28, 2017, 8:57 p.m. UTC | #1 
2017-10-17 23:42 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>:

> Attached patch is useful in combination with the transparency patch
> for paletteuse.

> +    { "background", "set a background color for transparency", OFFSET(background), AV_OPT_TYPE_COLOR, {.str="white"}, CHAR_MIN, CHAR_MAX, FLAGS },

Locally changed to "lime" to make sure fate stays unchanged.

Ping, will probably commit if there are no objections.

Carl Eugen
Clément Bœsch Oct. 28, 2017, 9:50 p.m. UTC | #2 
On Sat, Oct 28, 2017 at 10:57:32PM +0200, Carl Eugen Hoyos wrote:
> 2017-10-17 23:42 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> 
> > Attached patch is useful in combination with the transparency patch
> > for paletteuse.
> 
> > +    { "background", "set a background color for transparency", OFFSET(background), AV_OPT_TYPE_COLOR, {.str="white"}, CHAR_MIN, CHAR_MAX, FLAGS },
> 
> Locally changed to "lime" to make sure fate stays unchanged.
> 

Please use "transparency_color" as option name (and field struct).

Also, the correct type in the structure is uint8_t[4]

And ideally, you would also document the option in doc/filters.texi

You can apply any time with these changes.

Thanks,
Carl Eugen Hoyos Oct. 28, 2017, 11:47 p.m. UTC | #3 
2017-10-28 23:50 GMT+02:00 Clément Bœsch <u@pkh.me>:
> On Sat, Oct 28, 2017 at 10:57:32PM +0200, Carl Eugen Hoyos wrote:
>> 2017-10-17 23:42 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>>
>> > Attached patch is useful in combination with the transparency patch
>> > for paletteuse.
>>
>> > +    { "background", "set a background color for transparency", OFFSET(background), AV_OPT_TYPE_COLOR, {.str="white"}, CHAR_MIN, CHAR_MAX, FLAGS },
>>
>> Locally changed to "lime" to make sure fate stays unchanged.
>>
>
> Please use "transparency_color" as option name (and field struct).

I liked "background";-(

> Also, the correct type in the structure is uint8_t[4]
>
> And ideally, you would also document the option in doc/filters.texi
>
> You can apply any time with these changes.

Done, thank you!

Carl Eugen
James Almer Oct. 29, 2017, 12:32 a.m. UTC | #4 
On 10/28/2017 8:47 PM, Carl Eugen Hoyos wrote:
> 2017-10-28 23:50 GMT+02:00 Clément Bœsch <u@pkh.me>:
>> On Sat, Oct 28, 2017 at 10:57:32PM +0200, Carl Eugen Hoyos wrote:
>>> 2017-10-17 23:42 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>>>
>>>> Attached patch is useful in combination with the transparency patch
>>>> for paletteuse.
>>>
>>>> +    { "background", "set a background color for transparency", OFFSET(background), AV_OPT_TYPE_COLOR, {.str="white"}, CHAR_MIN, CHAR_MAX, FLAGS },
>>>
>>> Locally changed to "lime" to make sure fate stays unchanged.
>>>
>>
>> Please use "transparency_color" as option name (and field struct).
> 
> I liked "background";-(
> 
>> Also, the correct type in the structure is uint8_t[4]
>>
>> And ideally, you would also document the option in doc/filters.texi
>>
>> You can apply any time with these changes.
> 
> Done, thank you!
> 
> Carl Eugen

> diff --git a/libavfilter/vf_palettegen.c b/libavfilter/vf_palettegen.c
> index 03de317..2a04ae5 100644 (file)
> --- a/libavfilter/vf_palettegen.c
> +++ b/libavfilter/vf_palettegen.c
> @@ -27,6 +27,7 @@
>  #include "libavutil/internal.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/qsort.h"
> +#include "libavutil/intreadwrite.h"
>  #include "avfilter.h"
>  #include "internal.h"
>  
> @@ -74,6 +75,7 @@ typedef struct PaletteGenContext {
>      struct range_box boxes[256];            // define the segmentation of the colorspace (the final palette)
>      int nb_boxes;                           // number of boxes (increase will segmenting them)
>      int palette_pushed;                     // if the palette frame is pushed into the outlink or not
> +    uint8_t[4] transparency_color;          // background color for transparency

This broke compilation. It should be, if anything

uint8_t transparency_color[4];

>  } PaletteGenContext;
>  
>  #define OFFSET(x) offsetof(PaletteGenContext, x)
> @@ -81,6 +83,7 @@ typedef struct PaletteGenContext {
>  static const AVOption palettegen_options[] = {
>      { "max_colors", "set the maximum number of colors to use in the palette", OFFSET(max_colors), AV_OPT_TYPE_INT, {.i64=256}, 4, 256, FLAGS },
>      { "reserve_transparent", "reserve a palette entry for transparency", OFFSET(reserve_transparent), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, FLAGS },
> +    { "transparency_color", "set a background color for transparency", OFFSET(transparency_color), AV_OPT_TYPE_COLOR, {.str="lime"}, CHAR_MIN, CHAR_MAX, FLAGS },
>      { "stats_mode", "set statistics mode", OFFSET(stats_mode), AV_OPT_TYPE_INT, {.i64=STATS_MODE_ALL_FRAMES}, 0, NB_STATS_MODE-1, FLAGS, "mode" },
>          { "full", "compute full frame histograms", 0, AV_OPT_TYPE_CONST, {.i64=STATS_MODE_ALL_FRAMES}, INT_MIN, INT_MAX, FLAGS, "mode" },
>          { "diff", "compute histograms only for the part that differs from previous frame", 0, AV_OPT_TYPE_CONST, {.i64=STATS_MODE_DIFF_FRAMES}, INT_MIN, INT_MAX, FLAGS, "mode" },
> @@ -250,7 +253,7 @@ static void write_palette(AVFilterContext *ctx, AVFrame *out)
>  
>      if (s->reserve_transparent) {
>          av_assert0(s->nb_boxes < 256);
> -        pal[out->width - pal_linesize - 1] = 0x0000ff00; // add a green transparent color
> +        pal[out->width - pal_linesize - 1] = AV_RB32(&s->transparency_color) >> 8;
>      }
>  }
Carl Eugen Hoyos Oct. 29, 2017, 12:37 a.m. UTC | #5 
2017-10-29 2:32 GMT+02:00 James Almer <jamrial@gmail.com>:
> On 10/28/2017 8:47 PM, Carl Eugen Hoyos wrote:

>> +    uint8_t[4] transparency_color;          // background color for transparency
>
> This broke compilation

Should be fixed, sorry, I was already testing yuva2rgb on BE and
forgot to compile on my desktop.

Carl Eugen
Clément Bœsch Oct. 29, 2017, 4:25 p.m. UTC | #6 
On Sun, Oct 29, 2017 at 01:47:03AM +0200, Carl Eugen Hoyos wrote:
> 2017-10-28 23:50 GMT+02:00 Clément Bœsch <u@pkh.me>:
> > On Sat, Oct 28, 2017 at 10:57:32PM +0200, Carl Eugen Hoyos wrote:
> >> 2017-10-17 23:42 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> >>
> >> > Attached patch is useful in combination with the transparency patch
> >> > for paletteuse.
> >>
> >> > +    { "background", "set a background color for transparency", OFFSET(background), AV_OPT_TYPE_COLOR, {.str="white"}, CHAR_MIN, CHAR_MAX, FLAGS },
> >>
> >> Locally changed to "lime" to make sure fate stays unchanged.
> >>
> >
> > Please use "transparency_color" as option name (and field struct).
> 
> I liked "background";-(
> 

But it's not a background color, it's a reference color code typically
used for the transparency optimization. It's a reserved entry such that
the encoder can perform a good compression.
diff mbox

Patch

From fc1c562748b2f7bb518df9feaa91482757ae9625 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Tue, 17 Oct 2017 23:39:59 +0200
Subject: [PATCH] lavfi/palettegen: Allow setting the background colour.

---
 libavfilter/vf_palettegen.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavfilter/vf_palettegen.c b/libavfilter/vf_palettegen.c
index 03de317..0c74af6 100644
--- a/libavfilter/vf_palettegen.c
+++ b/libavfilter/vf_palettegen.c
@@ -27,6 +27,7 @@ 
 #include "libavutil/internal.h"
 #include "libavutil/opt.h"
 #include "libavutil/qsort.h"
+#include "libavutil/intreadwrite.h"
 #include "avfilter.h"
 #include "internal.h"
 
@@ -74,6 +75,7 @@  typedef struct PaletteGenContext {
     struct range_box boxes[256];            // define the segmentation of the colorspace (the final palette)
     int nb_boxes;                           // number of boxes (increase will segmenting them)
     int palette_pushed;                     // if the palette frame is pushed into the outlink or not
+    uint32_t background;                    // background color for transparency
 } PaletteGenContext;
 
 #define OFFSET(x) offsetof(PaletteGenContext, x)
@@ -81,6 +83,7 @@  typedef struct PaletteGenContext {
 static const AVOption palettegen_options[] = {
     { "max_colors", "set the maximum number of colors to use in the palette", OFFSET(max_colors), AV_OPT_TYPE_INT, {.i64=256}, 4, 256, FLAGS },
     { "reserve_transparent", "reserve a palette entry for transparency", OFFSET(reserve_transparent), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, FLAGS },
+    { "background", "set a background color for transparency", OFFSET(background), AV_OPT_TYPE_COLOR, {.str="white"}, CHAR_MIN, CHAR_MAX, FLAGS },
     { "stats_mode", "set statistics mode", OFFSET(stats_mode), AV_OPT_TYPE_INT, {.i64=STATS_MODE_ALL_FRAMES}, 0, NB_STATS_MODE-1, FLAGS, "mode" },
         { "full", "compute full frame histograms", 0, AV_OPT_TYPE_CONST, {.i64=STATS_MODE_ALL_FRAMES}, INT_MIN, INT_MAX, FLAGS, "mode" },
         { "diff", "compute histograms only for the part that differs from previous frame", 0, AV_OPT_TYPE_CONST, {.i64=STATS_MODE_DIFF_FRAMES}, INT_MIN, INT_MAX, FLAGS, "mode" },
@@ -250,7 +253,7 @@  static void write_palette(AVFilterContext *ctx, AVFrame *out)
 
     if (s->reserve_transparent) {
         av_assert0(s->nb_boxes < 256);
-        pal[out->width - pal_linesize - 1] = 0x0000ff00; // add a green transparent color
+        pal[out->width - pal_linesize - 1] = AV_RB32(&s->background) >> 8;
     }
 }
 
-- 
1.7.10.4