[FFmpeg-devel] [PATCH v2] lavc/libaribcaption.c: add MSZ characters related options
Jan Ekström
jeebjp at gmail.com
Thu Oct 12 00:32:29 EEST 2023
On Tue, Oct 10, 2023 at 1:29 PM TADANO Tokumei <aimingoff at pc.nifty.jp> wrote:
>
> ping!
>
> On 2023/09/29 19:31, TADANO Tokumei wrote:
> >
> > On 2023/09/23 11:15, TADANO Tokumei wrote:
> >> This is updated patch to "[PATCH] lavc/libaribcaption.c: add -replace_fullwidth_japanese option" (Message-Id: <20230908130050.85688-1-aimingoff at pc.nifty.jp>).
> >>
> >> If specified fonts contain half-width glyphs, it make better rendering with
> >> `-replace_msz_ascii false -replace_msz_japanese false` option for bitmap sub_type.
> >>
> >> This patch also fix a bug in libaribcaption.c. I prefer to apply this ASAP.
> >>
> >> On 2023/09/23 11:00, TADANO Tokumei wrote:
> >>> This patch add MSZ (Middle Size; half width) characters related
> >>> options.
> >>>
> >>> * add `-replace_msz_japanese` option introduced in version 1.0.1
> >>> of libaribcaption.
> >>> * add `-replace_msz_glyph` option introduced in version 1.1.0
> >>> of libaribcaption.
> >>> * rename `-replace_fullwidth_ascii` option to `-replace_msz_ascii`
> >>> to clarify option meaning.
> >>> * FIX: change all the `bool` option variables in `ARIBCaptionContext`
> >>> to `int`.
> >>> On some environments, a `bool` variable is small space than `int`.
> >>> If a `bool` option was specified by command line, following
> >>> variables would be filled and destroyed.
> >>> ---
> >>> doc/decoders.texi | 28 ++++++++++++++++++++------
> >>> libavcodec/libaribcaption.c | 40 ++++++++++++++++++++++++++++---------
> >>> 2 files changed, 53 insertions(+), 15 deletions(-)
> >>>
> >>> diff --git a/doc/decoders.texi b/doc/decoders.texi
> >>> index 09b8314dd2..27b981b267 100644
> >>> --- a/doc/decoders.texi
> >>> +++ b/doc/decoders.texi
> >>> @@ -427,12 +427,6 @@ If your player cannot handle AVSubtitles with multiple ASS rectangles properly,
> >>> set this option to @var{true} or define @env{ASS_SINGLE_RECT=1} to change
> >>> default behavior at compilation.
> >>> - at item -replace_fullwidth_ascii @var{boolean}
> >>> -Specify whether to replace MSZ (Middle Size, half width) fullwidth
> >>> -alphanumerics with halfwidth alphanumerics.
> >>> -
> >>> -The default is @var{true}.
> >>> -
> >>> @item -force_outline_text @var{boolean}
> >>> Specify whether always render outline text for all characters regardless of
> >>> the indication by charactor style.
> >>> @@ -459,6 +453,28 @@ Specify whether to render replaced DRCS characters as Unicode characters.
> >>> The default is @var{true}.
> >>> + at item -replace_msz_ascii @var{boolean}
> >>> +Specify whether to replace MSZ (Middle Size, half width) fullwidth
> >>> +alphanumerics with halfwidth alphanumerics.
> >>> +
> >>> +The default is @var{true}.
> >>> +
> >>> + at item -replace_msz_japanese @var{boolean}
> >>> +Specify whether to replace some MSZ (Middle Size, half width) fullwidth
> >>> +japanese special characters with halfwidth ones.
> >>> +
> >>> +The default is @var{true}.
> >>> +
> >>> + at item -replace_msz_glyph @var{boolean}
> >>> +Specify whether to replace MSZ (Middle Size, half width) characters
> >>> +with halfwidth glyphs if the fonts supports it.
> >>> +This option works under FreeType or DirectWrite renderer
> >>> +with Adobe-Japan1 compliant fonts.
> >>> +e.g., IBM Plex Sans JP, Morisawa BIZ UDGothic, Morisawa BIZ UDMincho,
> >>> +Yu Gothic, Yu Mincho, and Meiryo.
> >>> +
> >>> +The default is @var{true}.
> >>> +
> >>> @item -canvas_size @var{image_size}
> >>> Specify the resolution of the canvas to render subtitles to; usually, this
> >>> should be frame size of input video.
> >>> diff --git a/libavcodec/libaribcaption.c b/libavcodec/libaribcaption.c
> >>> index 8a8c8f8cfd..ddff47633a 100644
> >>> --- a/libavcodec/libaribcaption.c
> >>> +++ b/libavcodec/libaribcaption.c
> >>> @@ -68,14 +68,20 @@ typedef struct ARIBCaptionContext {
> >>> int subtitle_type;
> >>> int encoding_scheme;
> >>> - bool ass_single_rect;
> >>> + int ass_single_rect;
> >>> char *font;
> >>> - bool replace_fullwidth_ascii;
> >>> - bool force_stroke_text;
> >>> - bool ignore_background;
> >>> - bool ignore_ruby;
> >>> + int force_stroke_text;
> >>> + int ignore_background;
> >>> + int ignore_ruby;
> >>> float stroke_width;
> >>> - bool replace_drcs;
> >>> + int replace_drcs;
> >>> + int replace_msz_ascii;
> >>> +#if defined(ARIBCC_VERSION)
> >>> + int replace_msz_japanese;
> >>> +# if AV_VERSION_INT(ARIBCC_VERSION_MAJOR, ARIBCC_VERSION_MINOR, ARIBCC_VERSION_PATCH) >= AV_VERSION_INT(1, 1, 0)
> >>> + int replace_msz_glyph;
> >>> +# endif
> >>> +#endif
> >>> int64_t pts;
> >>> AVRational time_base;
> >>> @@ -1004,7 +1010,11 @@ static int aribcaption_init(AVCodecContext *avctx)
> >>> return AVERROR_EXTERNAL;
> >>> }
> >>> aribcc_decoder_set_replace_msz_fullwidth_ascii(ctx->decoder,
> >>> - ctx->replace_fullwidth_ascii);
> >>> + ctx->replace_msz_ascii);
> >>> +#if defined(ARIBCC_VERSION)
> >>> + aribcc_decoder_set_replace_msz_fullwidth_japanese(ctx->decoder,
> >>> + ctx->replace_msz_japanese);
> >>> +#endif
> >>> /* Similar behavior as ffmpeg tool to set canvas size */
> >>> if (ctx->canvas_width > 0 && ctx->canvas_height > 0 &&
> >>> @@ -1057,6 +1067,10 @@ static int aribcaption_init(AVCodecContext *avctx)
> >>> aribcc_renderer_set_force_no_background(ctx->renderer, ctx->ignore_background);
> >>> aribcc_renderer_set_force_no_ruby(ctx->renderer, ctx->ignore_ruby);
> >>> aribcc_renderer_set_stroke_width(ctx->renderer, ctx->stroke_width);
> >>> +#if defined(ARIBCC_VERSION) && AV_VERSION_INT(ARIBCC_VERSION_MAJOR, ARIBCC_VERSION_MINOR, ARIBCC_VERSION_PATCH) >= AV_VERSION_INT(1, 1, 0)
> >>> + aribcc_renderer_set_replace_msz_halfwidth_glyph(ctx->renderer,
> >>> + ctx->replace_msz_glyph);
> >>> +#endif
> >>> if (ctx->font) {
> >>> int is_nomem = 0;
> >>> size_t count = 0;
> >>> @@ -1132,8 +1146,6 @@ static const AVOption options[] = {
> >>> OFFSET(ass_single_rect), AV_OPT_TYPE_BOOL, { .i64 = ASS_SINGLE_RECT }, 0, 1, SD },
> >>> { "font", "comma-separated font family [ass, bitmap]",
> >>> OFFSET(font), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, SD },
> >>> - { "replace_fullwidth_ascii", "replace MSZ fullwidth alphanumerics with halfwidth alphanumerics [ass, bitmap]",
> >>> - OFFSET(replace_fullwidth_ascii), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, SD },
> >>> { "force_outline_text", "always render characters with outline [(ass), bitmap]",
> >>> OFFSET(force_stroke_text), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, SD },
> >>> { "ignore_background", "ignore rendering caption background [(ass), bitmap]",
> >>> @@ -1144,6 +1156,16 @@ static const AVOption options[] = {
> >>> OFFSET(stroke_width), AV_OPT_TYPE_FLOAT, { .dbl = 1.5 }, 0.0, 3.0, SD },
> >>> { "replace_drcs", "replace known DRCS [bitmap]",
> >>> OFFSET(replace_drcs), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, SD },
> >>> + { "replace_msz_ascii", "replace MSZ fullwidth alphanumerics with halfwidth alphanumerics [ass, bitmap]",
> >>> + OFFSET(replace_msz_ascii), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, SD },
> >>> +#if defined(ARIBCC_VERSION)
> >>> + { "replace_msz_japanese", "replace MSZ fullwidth Japanese with halfwidth [ass, bitmap]",
> >>> + OFFSET(replace_msz_japanese), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, SD },
> >>> +# if AV_VERSION_INT(ARIBCC_VERSION_MAJOR, ARIBCC_VERSION_MINOR, ARIBCC_VERSION_PATCH) >= AV_VERSION_INT(1, 1, 0)
> >>> + { "replace_msz_glyph", "replace MSZ characters with halfwidth glyphs [bitmap]",
> >>> + OFFSET(replace_msz_glyph), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, SD },
> >>> +# endif
> >>> +#endif
> >>> {"canvas_size", "set input video size (WxH or abbreviation) [bitmap]",
> >>> OFFSET(canvas_width), AV_OPT_TYPE_IMAGE_SIZE, { .str = NULL }, 0, INT_MAX, SD },
> >>> { NULL }
> >
> > ping!
> > Please push the patch if nobody has objections.
>
> If nobody has objections, could anyone push the patch set?
Took an initial look at this set.
Confirmed that it still builds with libaribcaption's first
compile-capable post-0.1.0 version
(06a695264f65a753c4feb300d9e8879248908b20), so the #ifs seem to work.
Some minor things:
- Generally we have a single commit contain a single change. this
seems to have three: 1) bool->int 2) existing option's rename to msz
3) addition of new msz options for newer libaribcaption versions? So
these should be split.
- As this wrapper is pretty new, depending on how much debugging of
old libaribcaption versions is seen as useful, it might make sense to
just bump the required version to 1.1.0 in configure, and drop the
#ifs to simplify the wrapper.
- Generally if wrappers have been around for a while, options should
be deprecated before being removed. Given that this wrapper has not
yet been in any release (it was merged after 6.0, and 6.1 is not
released yet), it might be OK to just rename an option? Not sure.
Jan
More information about the ffmpeg-devel
mailing list