[FFmpeg-devel] [PATCH] lavc/dvdsubenc: accept palette from options
Nicolas George
george at nsup.org
Sat Jan 25 16:13:19 EET 2020
Michael Kuron (12020-01-25):
> 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 at gmail.com>
> ---
> doc/decoders.texi | 2 +-
> doc/encoders.texi | 8 ++++++++
> libavcodec/Makefile | 1 +
> libavcodec/dvdsub.c | 35 +++++++++++++++++++++++++++++++++++
> libavcodec/dvdsub.h | 29 +++++++++++++++++++++++++++++
> libavcodec/dvdsubdec.c | 23 +++++++----------------
> libavcodec/dvdsubenc.c | 9 ++++++++-
> 7 files changed, 89 insertions(+), 18 deletions(-)
> create mode 100644 libavcodec/dvdsub.c
> create mode 100644 libavcodec/dvdsub.h
Thanks for the update.
In principle, we like to have two patches in this kind of cases: first
update the existing code to make a separate function; second add the new
feature.
>
> diff --git a/doc/decoders.texi b/doc/decoders.texi
> index 0582b018b0..83515ae363 100644
> --- a/doc/decoders.texi
> +++ b/doc/decoders.texi
> @@ -280,7 +280,7 @@ palette is stored in the IFO file, and therefore not available when reading
> from dumped VOB files.
>
> The format for this option is a string containing 16 24-bits hexadecimal
> -numbers (without 0x prefix) separated by comas, for example @code{0d00ee,
> +numbers (without 0x prefix) separated by commas, for example @code{0d00ee,
This is unrelated, but I personally think we can live with that.
> ee450d, 101010, eaeaea, 0ce60b, ec14ed, ebff0b, 0d617a, 7b7b7b, d1d1d1,
> 7b2a0e, 0d950c, 0f007b, cf0dec, cfa80c, 7c127b}.
>
> 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
> + at 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/Makefile b/libavcodec/Makefile
> index 3cd73fbcc6..07a05bd10b 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -283,6 +283,7 @@ OBJS-$(CONFIG_DVBSUB_DECODER) += dvbsubdec.o
> OBJS-$(CONFIG_DVBSUB_ENCODER) += dvbsub.o
> OBJS-$(CONFIG_DVDSUB_DECODER) += dvdsubdec.o
> OBJS-$(CONFIG_DVDSUB_ENCODER) += dvdsubenc.o
> +OBJS-$(CONFIG_DVDSUB_ENCODER) += dvdsub.o
I suspect you need the same line for CONFIG_DVDSUB_DECODER.
> OBJS-$(CONFIG_DVAUDIO_DECODER) += dvaudiodec.o
> OBJS-$(CONFIG_DVVIDEO_DECODER) += dvdec.o dv.o dvdata.o
> OBJS-$(CONFIG_DVVIDEO_ENCODER) += dvenc.o dv.o dvdata.o
> diff --git a/libavcodec/dvdsub.c b/libavcodec/dvdsub.c
> new file mode 100644
> index 0000000000..687af43590
> --- /dev/null
> +++ b/libavcodec/dvdsub.c
> @@ -0,0 +1,35 @@
> +/*
> + * DVD subtitle decoding/encoding
> + * Copyright (c) 2005 Fabrice Bellard
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "dvdsub.h"
> +#include "libavutil/avstring.h"
> +#include <stdlib.h>
> +
> +void ff_dvdsub_parse_palette(uint32_t *palette, char *p)
const char *
> +{
> + int i;
> +
> + for (i=0; i<16; i++) {
Some spaces after comparisons.
(This is not your code, but since it is moving now, we can clean the
cosmetics at the same time.)
> + palette[i] = strtoul(p, &p, 16);
> + while (*p == ',' || av_isspace(*p))
> + p++;
> + }
> +}
> diff --git a/libavcodec/dvdsub.h b/libavcodec/dvdsub.h
> new file mode 100644
> index 0000000000..908589709a
> --- /dev/null
> +++ b/libavcodec/dvdsub.h
I think you can put this in internal.h.
> @@ -0,0 +1,29 @@
> +/*
> + * DVD subtitle decoding/encoding
> + * Copyright (c) 2005 Fabrice Bellard
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVCODEC_DVDSUB_H
> +#define AVCODEC_DVDSUB_H
> +
> +#include <inttypes.h>
> +
> +void ff_dvdsub_parse_palette(uint32_t *palette, char *p);
> +
> +#endif /* AVCODEC_DVDSUB_H */
> \ No newline at end of file
> diff --git a/libavcodec/dvdsubdec.c b/libavcodec/dvdsubdec.c
> index 741ea9fd1e..856eb3dcc9 100644
> --- a/libavcodec/dvdsubdec.c
> +++ b/libavcodec/dvdsubdec.c
> @@ -20,6 +20,7 @@
> */
>
> #include "avcodec.h"
> +#include "dvdsub.h"
> #include "get_bits.h"
> #include "internal.h"
>
> @@ -27,7 +28,6 @@
> #include "libavutil/colorspace.h"
> #include "libavutil/opt.h"
> #include "libavutil/imgutils.h"
> -#include "libavutil/avstring.h"
> #include "libavutil/bswap.h"
>
> typedef struct DVDSubContext
> @@ -626,18 +626,6 @@ static int dvdsub_decode(AVCodecContext *avctx,
> return buf_size;
> }
>
> -static void parse_palette(DVDSubContext *ctx, char *p)
> -{
> - int i;
> -
> - ctx->has_palette = 1;
> - for(i=0;i<16;i++) {
> - ctx->palette[i] = strtoul(p, &p, 16);
> - while(*p == ',' || av_isspace(*p))
> - p++;
> - }
> -}
> -
> static int parse_ifo_palette(DVDSubContext *ctx, char *p)
> {
> FILE *ifo;
> @@ -719,7 +707,8 @@ static int dvdsub_parse_extradata(AVCodecContext *avctx)
> break;
>
> if (strncmp("palette:", data, 8) == 0) {
> - parse_palette(ctx, data + 8);
> + ctx->has_palette = 1;
> + ff_dvdsub_parse_palette(ctx->palette, data + 8);
> } else if (strncmp("size:", data, 5) == 0) {
> int w, h;
> if (sscanf(data + 5, "%dx%d", &w, &h) == 2) {
> @@ -748,8 +737,10 @@ static av_cold int dvdsub_init(AVCodecContext *avctx)
>
> if (ctx->ifo_str)
> parse_ifo_palette(ctx, ctx->ifo_str);
> - if (ctx->palette_str)
> - parse_palette(ctx, ctx->palette_str);
> + if (ctx->palette_str) {
> + ctx->has_palette = 1;
> + ff_dvdsub_parse_palette(ctx->palette, ctx->palette_str);
> + }
> if (ctx->has_palette) {
> int i;
> av_log(avctx, AV_LOG_DEBUG, "palette:");
> diff --git a/libavcodec/dvdsubenc.c b/libavcodec/dvdsubenc.c
> index ff95ed2002..61dd6c48fc 100644
> --- a/libavcodec/dvdsubenc.c
> +++ b/libavcodec/dvdsubenc.c
> @@ -19,6 +19,7 @@
> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> */
> #include "avcodec.h"
> +#include "dvdsub.h"
> #include "bytestream.h"
> #include "internal.h"
> #include "libavutil/avassert.h"
> @@ -29,6 +30,7 @@
> typedef struct {
> AVClass *class;
> uint32_t global_palette[16];
> + char *palette_str;
I don't think alignment is very useful here.
> int even_rows_fix;
> } DVDSubtitleContext;
>
> @@ -436,7 +438,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) {
> + ff_dvdsub_parse_palette(dvdc->global_palette, 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 +473,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 },
> {"even_rows_fix", "Make number of rows even (workaround for some players)", OFFSET(even_rows_fix), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SE},
> { NULL },
> };
Regards,
--
Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200125/1788a5ff/attachment.sig>
More information about the ffmpeg-devel
mailing list