Re: [FFmpeg-devel] [PATCH] avfilter/vf_subtitles: add wrap_unicode option

["Zhao Zhili" <quinkblack@foxmail.com>]

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Lance Wang
> Sent: 2023年5月19日 22:15
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] avfilter/vf_subtitles: add wrap_unicode option
> 
> On Fri, May 19, 2023 at 3:59 PM Zhao Zhili <quinkblack@foxmail.com> wrote:
> 
> > From: Zhao Zhili <zhilizhao@tencent.com>
> >
> > So CJK can be wrapped automatically.
> >
> > Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
> > ---
> >  libavfilter/version.h      |  2 +-
> >  libavfilter/vf_subtitles.c | 13 +++++++++++++
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavfilter/version.h b/libavfilter/version.h
> > index ba8a6fdab2..08779130f5 100644
> > --- a/libavfilter/version.h
> > +++ b/libavfilter/version.h
> > @@ -32,7 +32,7 @@
> >  #include "version_major.h"
> >
> >  #define LIBAVFILTER_VERSION_MINOR   8
> > -#define LIBAVFILTER_VERSION_MICRO 100
> > +#define LIBAVFILTER_VERSION_MICRO 101
> >
> >
> >  #define LIBAVFILTER_VERSION_INT AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR,
> > \
> > diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
> > index 82e140e986..d6a68e5cdd 100644
> > --- a/libavfilter/vf_subtitles.c
> > +++ b/libavfilter/vf_subtitles.c
> > @@ -61,6 +61,7 @@ typedef struct AssContext {
> >      int original_w, original_h;
> >      int shaping;
> >      FFDrawContext draw;
> > +    int wrap_unicode;
> >  } AssContext;
> >
> >  #define OFFSET(x) offsetof(AssContext, x)
> > @@ -72,6 +73,7 @@ typedef struct AssContext {
> >      {"original_size",  "set the size of the original video (used to scale
> > fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL},  0, 0,
> > FLAGS }, \
> >      {"fontsdir",       "set the directory containing the fonts to read",
> >          OFFSET(fontsdir),   AV_OPT_TYPE_STRING,     {.str = NULL},  0, 0,
> > FLAGS }, \
> >      {"alpha",          "enable processing of alpha channel",
> >          OFFSET(alpha),      AV_OPT_TYPE_BOOL,       {.i64 = 0   },
> >  0,        1, FLAGS }, \
> > +    {"wrap_unicode",   "break lines according to the Unicode Line
> > Breaking Algorithm", OFFSET(wrap_unicode), AV_OPT_TYPE_BOOL, {.i64 = 1
> >  },         0,        1, FLAGS }, \
> >
> >  /* libass supports a log level ranging from 0 to 7 */
> >  static const int ass_libavfilter_log_level_map[] = {
> > @@ -329,6 +331,17 @@ static av_cold int init_subtitles(AVFilterContext
> > *ctx)
> >          return AVERROR(EINVAL);
> >      }
> >
> > +    if (ass->wrap_unicode) {
> > +#if (LIBASS_VERSION >= 0x01600010)
> > +        ret = ass_track_set_feature(ass->track, ASS_FEATURE_WRAP_UNICODE,
> > 1);
> > +        if (ret < 0)
> > +            av_log(ctx, AV_LOG_WARNING, "libass doesn't build with
> > ASS_FEATURE_WRAP_UNICODE support\n");
> > +#else
> > +        av_log(ctx, AV_LOG_INFO, "libass version %#x doesn't support
> > ASS_FEATURE_WRAP_UNICODE\n",
> > +               LIBASS_VERSION);
> > +#endif
> > +    }
> > +
> >
> 
> I prefer to enable the feature default(if libass library support) instead
> of adding a new option to turn off it(default is on).
> Also, if the version is right, then why to check the return?

User may wants to scale fonts than wrap, for example. I'm not familiar with
the use case, since libass gives it as an option, I guess there is a reason for that.

To support ASS_FEATURE_WRAP_UNICODE, libass depends on libunibreak. So it
can fail if libass doesn't build with libunibreak enabled. The log messages revealed
two different failures.

> +#if (LIBASS_VERSION >= 0x01600010)
> +        ass_track_set_feature(ass->track, ASS_FEATURE_WRAP_UNICODE, 1);
> +#endif
> +
> 
> 
> >      /* Open subtitles file */
> >      ret = avformat_open_input(&fmt, ass->filename, NULL, NULL);
> >      if (ret < 0) {
> > --
> > 2.25.1
> >
> > _______________________________________________
> > 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 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 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".