[FFmpeg-devel] [PATCH v4 0/3] lavc/libaribcaption.c: add MSZ characters related options

[FFmpeg-devel] [PATCH v4 0/3] lavc/libaribcaption.c: add MSZ characters related options

[TADANO Tokumei]

Re-send the patchset.
The patches are same as v3, but its thread were corrupted.

-----
This patch add MSZ (Middle Size; half width) characters related
options and some fixes.

As Jan suggested to PATCH v2, I split it into 3 patches, drop `#if`s, and
bump required libaribcaption version to 1.1.1 (author preferred version).

TADANO Tokumei (3):
  lavc/libaribcaption.c: FIX: change all `boot` option var to `int`
  lavc/libaribcaption.c: add MSZ characters related options
  lavc/libaribcaption.c: rename `-replace_fullwidth_ascii` option

 configure                   |  2 +-
 doc/decoders.texi           | 28 ++++++++++++++++++++++------
 libavcodec/libaribcaption.c | 28 +++++++++++++++++++---------
 3 files changed, 42 insertions(+), 16 deletions(-)

-- 
2.39.2

_______________________________________________
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".

[FFmpeg-devel] [PATCH v4 1/3] lavc/libaribcaption.c: FIX: change all `boot` option var to `int`

[TADANO Tokumei]

This patch fixes a bug in curret version.

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 may be destroyed by av_opt_copy().

This patch change all the `bool` option variables in
`ARIBCaptionContext` to `int`.

Signed-off-by: TADANO Tokumei <aimingoff@pc.nifty.jp>
---
 libavcodec/libaribcaption.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavcodec/libaribcaption.c b/libavcodec/libaribcaption.c
index 8a8c8f8cfd..be3328c5c9 100644
--- a/libavcodec/libaribcaption.c
+++ b/libavcodec/libaribcaption.c
@@ -68,14 +68,14 @@ 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 replace_fullwidth_ascii;
+    int force_stroke_text;
+    int ignore_background;
+    int ignore_ruby;
     float stroke_width;
-    bool replace_drcs;
+    int replace_drcs;
 
     int64_t pts;
     AVRational time_base;
-- 
2.39.2

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v4 1/3] lavc/libaribcaption.c: FIX: change all `boot` option var to `int`

[Jan Ekström]

On Tue, Oct 17, 2023 at 4:14 PM TADANO Tokumei <aimingoff@pc.nifty.jp> wrote:
>
> This patch fixes a bug in curret version.
>
> 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 may be destroyed by av_opt_copy().
>
> This patch change all the `bool` option variables in
> `ARIBCaptionContext` to `int`.
>
> Signed-off-by: TADANO Tokumei <aimingoff@pc.nifty.jp>

Change LGTM. I changed the commit message to more match our guide
lines (no ".c" at the end, rephrased the commit message) in
https://github.com/jeeb/ffmpeg/commits/aribcaption_msz_patches_v3 .

Just tell me if you're OK with this and I'll pull it in.

Jan
_______________________________________________
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".

[FFmpeg-devel] [PATCH v4 2/3] lavc/libaribcaption.c: add MSZ characters related options

[TADANO Tokumei]

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.

If specified fonts contain half-width glyphs (e.g., BIZ UDGothic),
it make better rendering with `-replace_msz_ascii false` and
`-replace_msz_japanese false` option for bitmap sub_type.

Signed-off-by: TADANO Tokumei <aimingoff@pc.nifty.jp>
---
 configure                   |  2 +-
 doc/decoders.texi           | 16 ++++++++++++++++
 libavcodec/libaribcaption.c | 10 ++++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index d203177a74..2fb827e1fb 100755
--- a/configure
+++ b/configure
@@ -6679,7 +6679,7 @@ enabled libaom            && require_pkg_config libaom "aom >= 1.0.0" aom/aom_co
 enabled libaribb24        && { check_pkg_config libaribb24 "aribb24 > 1.0.3" "aribb24/aribb24.h" arib_instance_new ||
                                { enabled gpl && require_pkg_config libaribb24 aribb24 "aribb24/aribb24.h" arib_instance_new; } ||
                                die "ERROR: libaribb24 requires version higher than 1.0.3 or --enable-gpl."; }
-enabled libaribcaption    && require_pkg_config libaribcaption "libaribcaption >= 0.1.0" "aribcaption/aribcaption.h" aribcc_context_alloc
+enabled libaribcaption    && require_pkg_config libaribcaption "libaribcaption >= 1.1.1" "aribcaption/aribcaption.h" aribcc_context_alloc
 enabled lv2               && require_pkg_config lv2 lilv-0 "lilv/lilv.h" lilv_world_new
 enabled libiec61883       && require libiec61883 libiec61883/iec61883.h iec61883_cmp_connect -lraw1394 -lavc1394 -lrom1394 -liec61883
 enabled libass            && require_pkg_config libass "libass >= 0.11.0" ass/ass.h ass_library_init
diff --git a/doc/decoders.texi b/doc/decoders.texi
index 09b8314dd2..36c3404475 100644
--- a/doc/decoders.texi
+++ b/doc/decoders.texi
@@ -459,6 +459,22 @@ Specify whether to render replaced DRCS characters as Unicode characters.
 
 The default is @var{true}.
 
+@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}.
+
+@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 be3328c5c9..74822586dc 100644
--- a/libavcodec/libaribcaption.c
+++ b/libavcodec/libaribcaption.c
@@ -76,6 +76,8 @@ typedef struct ARIBCaptionContext {
     int ignore_ruby;
     float stroke_width;
     int replace_drcs;
+    int replace_msz_japanese;
+    int replace_msz_glyph;
 
     int64_t pts;
     AVRational time_base;
@@ -1005,6 +1007,8 @@ static int aribcaption_init(AVCodecContext *avctx)
     }
     aribcc_decoder_set_replace_msz_fullwidth_ascii(ctx->decoder,
                                                    ctx->replace_fullwidth_ascii);
+    aribcc_decoder_set_replace_msz_fullwidth_japanese(ctx->decoder,
+                                                   ctx->replace_msz_japanese);
 
     /* Similar behavior as ffmpeg tool to set canvas size */
     if (ctx->canvas_width > 0 && ctx->canvas_height > 0 &&
@@ -1057,6 +1061,8 @@ 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);
+        aribcc_renderer_set_replace_msz_halfwidth_glyph(ctx->renderer,
+                                                        ctx->replace_msz_glyph);
         if (ctx->font) {
             int is_nomem = 0;
             size_t count = 0;
@@ -1144,6 +1150,10 @@ 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_japanese", "replace MSZ fullwidth Japanese with halfwidth [ass, bitmap]",
+      OFFSET(replace_msz_japanese), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, SD },
+    { "replace_msz_glyph", "replace MSZ characters with halfwidth glyphs [bitmap]",
+      OFFSET(replace_msz_glyph), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, SD },
     {"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 }
-- 
2.39.2

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v4 2/3] lavc/libaribcaption.c: add MSZ characters related options

[Jan Ekström]

On Tue, Oct 17, 2023 at 4:14 PM TADANO Tokumei <aimingoff@pc.nifty.jp> 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.
>
> If specified fonts contain half-width glyphs (e.g., BIZ UDGothic),
> it make better rendering with `-replace_msz_ascii false` and
> `-replace_msz_japanese false` option for bitmap sub_type.
>
> Signed-off-by: TADANO Tokumei <aimingoff@pc.nifty.jp>

Change-wise LGTM. I reworked the commit message (starting with the
removal of ".c" at the end), and hopefully I understood correctly what
you were trying to mean with what I expect you were meaning with
regards to the latter option. The adjusted commits are visible in
https://github.com/jeeb/ffmpeg/commits/aribcaption_msz_patches_v3 .

Otherwise the only thing I adjusted was

diff --git a/libavcodec/libaribcaption.c b/libavcodec/libaribcaption.c
index 2a058a4992..e87f303aa8 100644
--- a/libavcodec/libaribcaption.c
+++ b/libavcodec/libaribcaption.c
@@ -1008,7 +1008,7 @@ static int aribcaption_init(AVCodecContext *avctx)
     aribcc_decoder_set_replace_msz_fullwidth_ascii(ctx->decoder,
                                                    ctx->replace_msz_ascii);
     aribcc_decoder_set_replace_msz_fullwidth_japanese(ctx->decoder,
-                                                   ctx->replace_msz_japanese);
+
ctx->replace_msz_japanese);

     /* Similar behavior as ffmpeg tool to set canvas size */
     if (ctx->canvas_width > 0 && ctx->canvas_height > 0 &&

as there was a mismatching offset. If this is fine by you, I'll pull this in.

Jan
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v4 2/3] lavc/libaribcaption.c: add MSZ characters related options

[Jan Ekström]

On Sun, Oct 29, 2023 at 10:51 AM Jan Ekström <jeebjp@gmail.com> wrote:
>
> On Tue, Oct 17, 2023 at 4:14 PM TADANO Tokumei <aimingoff@pc.nifty.jp> 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.
> >
> > If specified fonts contain half-width glyphs (e.g., BIZ UDGothic),
> > it make better rendering with `-replace_msz_ascii false` and
> > `-replace_msz_japanese false` option for bitmap sub_type.
> >
> > Signed-off-by: TADANO Tokumei <aimingoff@pc.nifty.jp>
>
> Change-wise LGTM. I reworked the commit message (starting with the
> removal of ".c" at the end), and hopefully I understood correctly what
> you were trying to mean with what I expect you were meaning with
> regards to the latter option. The adjusted commits are visible in
> https://github.com/jeeb/ffmpeg/commits/aribcaption_msz_patches_v3 .
>
> Otherwise the only thing I adjusted was
>
> diff --git a/libavcodec/libaribcaption.c b/libavcodec/libaribcaption.c
> index 2a058a4992..e87f303aa8 100644
> --- a/libavcodec/libaribcaption.c
> +++ b/libavcodec/libaribcaption.c
> @@ -1008,7 +1008,7 @@ static int aribcaption_init(AVCodecContext *avctx)
>      aribcc_decoder_set_replace_msz_fullwidth_ascii(ctx->decoder,
>                                                     ctx->replace_msz_ascii);
>      aribcc_decoder_set_replace_msz_fullwidth_japanese(ctx->decoder,
> -                                                   ctx->replace_msz_japanese);
> +
> ctx->replace_msz_japanese);
>
>      /* Similar behavior as ffmpeg tool to set canvas size */
>      if (ctx->canvas_width > 0 && ctx->canvas_height > 0 &&
>
> as there was a mismatching offset. If this is fine by you, I'll pull this in.

Received an OK from the author regarding the commit message
adjustments and such privately, so will be merging this in.
_______________________________________________
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".

[FFmpeg-devel] [PATCH v4 3/3] lavc/libaribcaption.c: rename `-replace_fullwidth_ascii` option

[TADANO Tokumei]

This patch renames `-replace_fullwidth_ascii` option to
`-replace_msz_ascii` to clarify option meaning.

Signed-off-by: TADANO Tokumei <aimingoff@pc.nifty.jp>
---
 doc/decoders.texi           | 12 ++++++------
 libavcodec/libaribcaption.c |  8 ++++----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/doc/decoders.texi b/doc/decoders.texi
index 36c3404475..eb00e2a9e9 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.
 
-@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,12 @@ Specify whether to render replaced DRCS characters as Unicode characters.
 
 The default is @var{true}.
 
+@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}.
+
 @item -replace_msz_japanese @var{boolean}
 Specify whether to replace some MSZ (Middle Size; half width) fullwidth
 japanese special characters with halfwidth ones.
diff --git a/libavcodec/libaribcaption.c b/libavcodec/libaribcaption.c
index 74822586dc..2a058a4992 100644
--- a/libavcodec/libaribcaption.c
+++ b/libavcodec/libaribcaption.c
@@ -70,12 +70,12 @@ typedef struct ARIBCaptionContext {
     int encoding_scheme;
     int ass_single_rect;
     char *font;
-    int replace_fullwidth_ascii;
     int force_stroke_text;
     int ignore_background;
     int ignore_ruby;
     float stroke_width;
     int replace_drcs;
+    int replace_msz_ascii;
     int replace_msz_japanese;
     int replace_msz_glyph;
 
@@ -1006,7 +1006,7 @@ 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);
     aribcc_decoder_set_replace_msz_fullwidth_japanese(ctx->decoder,
                                                    ctx->replace_msz_japanese);
 
@@ -1138,8 +1138,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]",
@@ -1150,6 +1148,8 @@ 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 },
     { "replace_msz_japanese", "replace MSZ fullwidth Japanese with halfwidth [ass, bitmap]",
       OFFSET(replace_msz_japanese), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, SD },
     { "replace_msz_glyph", "replace MSZ characters with halfwidth glyphs [bitmap]",
-- 
2.39.2

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v4 3/3] lavc/libaribcaption.c: rename `-replace_fullwidth_ascii` option

[Jan Ekström]

On Tue, Oct 17, 2023 at 4:14 PM TADANO Tokumei <aimingoff@pc.nifty.jp> wrote:
>
> This patch renames `-replace_fullwidth_ascii` option to
> `-replace_msz_ascii` to clarify option meaning.
>
> Signed-off-by: TADANO Tokumei <aimingoff@pc.nifty.jp>

Asked around and James seemed to be OK with just replacing this option
as this is a "niche" module and since it had not yet been in any
release. Thus LGTM change-wise.

Same things as with the rest of the set, did adjustments to the commit
message. If you're fine with it, I'll pull these in.

Jan
_______________________________________________
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".