[FFmpeg-devel] lavc/dvdsubenc: accept palette from options

Previously, the default palette would always be used.
Now, we can accept a custom palette, just like dvdsubdec does.

Signed-off-by: Michael Kuron <michael.kuron@gmail.com>
---
 doc/encoders.texi      |  8 ++++++++
 libavcodec/dvdsubenc.c | 19 ++++++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

Am Fr., 24. Jan. 2020 um 23:49 Uhr schrieb Michael Kuron
<michael.kuron@gmail.com>:
>
> Previously, the default palette would always be used.
> Now, we can accept a custom palette, just like dvdsubdec does.
>
> Signed-off-by: Michael Kuron <michael.kuron@gmail.com>
> ---
>  doc/encoders.texi      |  8 ++++++++
>  libavcodec/dvdsubenc.c | 19 ++++++++++++++++++-
>  2 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index eefd124751..a04f9f1b62 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -3116,6 +3116,14 @@ and they can also be used in Matroska files.
>  @subsection Options
>
>  @table @option
> +@item palette
> +Specify the global palette used by the bitmaps.
> +
> +The format for this option is a string containing 16 24-bits hexadecimal
> +numbers (without 0x prefix) separated by commas, for example @code{0d00ee,
> +ee450d, 101010, eaeaea, 0ce60b, ec14ed, ebff0b, 0d617a, 7b7b7b, d1d1d1,
> +7b2a0e, 0d950c, 0f007b, cf0dec, cfa80c, 7c127b}.
> +
>  @item even_rows_fix
>  When set to 1, enable a work-around that makes the number of pixel rows
>  even in all subtitles.  This fixes a problem with some players that
> diff --git a/libavcodec/dvdsubenc.c b/libavcodec/dvdsubenc.c
> index ff95ed2002..674d97ab9b 100644
> --- a/libavcodec/dvdsubenc.c
> +++ b/libavcodec/dvdsubenc.c
> @@ -29,6 +29,7 @@
>  typedef struct {
>      AVClass *class;
>      uint32_t global_palette[16];
> +    char    *palette_str;
>      int even_rows_fix;
>  } DVDSubtitleContext;
>
> @@ -423,6 +424,17 @@ fail:
>      return ret;
>  }
>
> +static void parse_palette(DVDSubtitleContext *ctx, char *p)
> +{
> +    int i;
> +
> +    for (i=0; i<16; i++) {
> +        ctx->global_palette[i] = strtoul(p, &p, 16);
> +        while (*p == ',' || av_isspace(*p))
> +            p++;
> +    }
> +}
> +
>  static int dvdsub_init(AVCodecContext *avctx)
>  {
>      DVDSubtitleContext *dvdc = avctx->priv_data;
> @@ -436,7 +448,11 @@ static int dvdsub_init(AVCodecContext *avctx)
>      int i, ret;
>
>      av_assert0(sizeof(dvdc->global_palette) == sizeof(default_palette));
> -    memcpy(dvdc->global_palette, default_palette, sizeof(dvdc->global_palette));
> +    if (dvdc->palette_str) {
> +        parse_palette(dvdc, dvdc->palette_str);
> +    } else {
> +        memcpy(dvdc->global_palette, default_palette, sizeof(dvdc->global_palette));
> +    }
>
>      av_bprint_init(&extradata, 0, AV_BPRINT_SIZE_AUTOMATIC);
>      if (avctx->width && avctx->height)
> @@ -467,6 +483,7 @@ static int dvdsub_encode(AVCodecContext *avctx,
>  #define OFFSET(x) offsetof(DVDSubtitleContext, x)
>  #define SE AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_ENCODING_PARAM
>  static const AVOption options[] = {
> +    {"palette", "set the global palette", OFFSET(palette_str), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, SE },

What happens if invalid values are passed as palette?

Carl Eugen

On Sat, Jan 25, 2020 at 01:24 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>> static const AVOption options[] = {
>> +    {"palette", "set the global palette", OFFSET(palette_str), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, SE },
>
> What happens if invalid values are passed as palette?

The string is parsed until it hits something invalid (i.e. anything
except for space, comma, or hex number) and all remaining palette
colors are set to black. The same happens if you pass fewer than 16
values. This is what dvdsubdec does too. In fact, this entire patch is
basically copy+paste from there.

Michael Kuron (12020-01-25):
>					   In fact, this entire patch is
> basically copy+paste from there.

Then please fix the patch so that the same code is used, not a copy of
it. Otherwise, enhancements or fixes would have to be duplicated too.

Regards,