[FFmpeg-devel] [PATCH] lavfi/subtitles: load attached fonts to libass.

Facundo Gaich facugaich at gmail.com
Wed Apr 9 00:39:39 CEST 2014


On Tue, Apr 8, 2014 at 6:56 PM, wm4 <nfxjfg at googlemail.com> wrote:

> On Tue, 8 Apr 2014 17:37:01 -0300
> Facundo Gaich <facugaich at gmail.com> wrote:
>
> > On Apr 8, 2014 10:07 AM, "wm4" <nfxjfg at googlemail.com> wrote:
> > >
> > > On Tue, 8 Apr 2014 14:42:52 +0200
> > > Clément Boesch <u at pkh.me> wrote:
> > >
> > > > On Mon, Apr 07, 2014 at 11:35:01PM -0300, Facundo Gaich wrote:
> > > > > Videos with complex typesetting usually have font files embedded
> > > > > as attachment streams. vf_subtitles now finds all attachment
> > > > > streams with a MIME type associated with fonts and loads them
> > > > > to libass so it can use them for rendering.
> > > > >
> > > > > The code was basically ported from mpv's loadfile.c at 929793be7
> > > > >
> > > > > Signed-off-by: Facundo Gaich <facugaich at gmail.com>
> > > > > ---
> > > > >  libavfilter/vf_subtitles.c | 53
> > +++++++++++++++++++++++++++++++++++++++++++++-
> > > > >  1 file changed, 52 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/libavfilter/vf_subtitles.c
> b/libavfilter/vf_subtitles.c
> > > > > index e44f61d..8586304 100644
> > > > > --- a/libavfilter/vf_subtitles.c
> > > > > +++ b/libavfilter/vf_subtitles.c
> > > > > @@ -249,11 +249,34 @@ static const AVOption subtitles_options[] = {
> > > > >      {NULL},
> > > > >  };
> > > > >
> > > > > +static const char *font_mimetypes[] = {
> > > > > +    "application/x-truetype-font",
> > > > > +    "application/vnd.ms-opentype",
> > > > > +    "application/x-font-ttf",
> > > > > +    "application/x-font",
> > > > > +    NULL
> > >
> > > Looks exactly like the font mime type list in mpv.
> >
> > Are you suggesting a copyright issue (mpv being GPLv2 or later)? I'm not
> > familiar with how these are usually handled when the code is so trivial
> (or
> > in general)
>
> No, IMO too trivial. But you didn't copy the comment about the x-font
> mime type.
>
>
Got it.


> > > Note that
> > > 'application/x-font' probably never existed officially (e.g. it's not
> > > in Haali's matroska demuxer), and is probably wrong. On the other hand,
> > > it's probably not harmful to have this entry.
> > >
> >
> > Considering I went with a much less tolerant version of the mpv code
> (i.e.
> > I don't check filename extension in the absence of mimetype) I guess I
> > could leave out that case too.
> >
> > > > > +};
> > > > > +
> > > > > +static int attachment_is_font(AVStream * st)
> > > > > +{
> > > > > +    const AVDictionaryEntry *tag = NULL;
> > > > > +
> > > > > +    tag = av_dict_get(st->metadata, "mimetype", NULL,
> > AV_DICT_MATCH_CASE);
> > > > > +
> > > > > +    if (tag) {
> > > > > +        for (int n = 0; font_mimetypes[n]; n++) {
> > > >
> > > > int should be out of the loop (it's not supported by all the
> compilers
> > > > FFmpeg supports).
> > > >
> > > > > +            if (strcmp(font_mimetypes[n], tag->value) == 0)
> > >
> > > I think mime types are usually not case sensitive, so this catches
> > > fonts with lowercase mime types only.
> > >
> >
> > Noted
> >
> > > > > +                return 1;
> > > > > +        }
> > > > > +    }
> > > > > +    return 0;
> > > > > +}
> > > > > +
> > > > >  AVFILTER_DEFINE_CLASS(subtitles);
> > > > >
> > > > >  static av_cold int init_subtitles(AVFilterContext *ctx)
> > > > >  {
> > > > > -    int ret, sid;
> > > > > +    int ret, sid, loaded_fonts;
> > > > >      AVDictionary *codec_opts = NULL;
> > > > >      AVFormatContext *fmt = NULL;
> > > > >      AVCodecContext *dec_ctx = NULL;
> > > > > @@ -293,6 +316,34 @@ static av_cold int
> > init_subtitles(AVFilterContext *ctx)
> > > > >      sid = ret;
> > > > >      st = fmt->streams[sid];
> > > > >
> > > > > +    /* Load attached fonts */
> > > > > +    for (int j = 0; j < fmt->nb_streams; j++) {
> > > >
> > > > ditto
> > > >
> > > > > +        AVStream *st = fmt->streams[j];
> > > > > +        if ((st->codec->codec_type == AVMEDIA_TYPE_ATTACHMENT) &&
> > > >
> > > > Extra parenthesis
> > > >
> > > > > +            attachment_is_font(st)) {
> > > > > +            const AVDictionaryEntry *tag = NULL;
> > > > > +            tag = av_dict_get(st->metadata, "filename", NULL,
> > > > > +                              AV_DICT_MATCH_CASE);
> > > > > +
> > > > > +            if (tag) {
> > > > > +                av_log(ctx, AV_LOG_DEBUG, "Loading attached font:
> > %s\n",
> > > > > +                       tag->value);
> > > > > +                ass_add_font(ass->library, tag->value,
> > > > > +                             st->codec->extradata,
> > > > > +                             st->codec->extradata_size);
> > > > > +                loaded_fonts = 1;
> > > > > +            } else {
> > > > > +                av_log(ctx, AV_LOG_WARNING,
> > > > > +                       "Font attachment has no filename,
> > ignored.\n");
> > > > > +            }
> > > > > +        }
> > > > > +    }
> > > > > +    if (loaded_fonts) {
> > > > > +        /* Ideally this would be ass_fonts_update, but it seems to
> > be broken
> > > > > +         * as of libass 0.11.1 */
> > > >
> > > > Can you add a bug id or something for reference in the comment?
> > >
> > > IMO not really a bug. ass_fonts_update() literally just calls
> > > FcConfigBuildFonts, i.e. it updates the system fonts.
> > > ass_set_fonts on the other hand adds fonts from ASS_Library to
> > > ASS_Renderer.
> > >
> > > (It's worth noticing that libass wants to remove the separation between
> > > ASS_Library and ASS_Renderer, so the need for the API call will
> > > probably be removed once that is done.)
> > >
> >
> > Maybe calling it broken was an overstatement, if indeed that's the
> intended
> > function of ass_fonts_update, the fault lies on a lack of
> documentation/API
> > clarity. Anyways, considering the API will change in the future, is it ok
> > if I change the comment to indicate that's the correct API call to force
> > reloading of in memory fonts?
>
> I guess so... By the way, I noticed that there's another ass_set_fonts()
> call in init(), and this code is shared between the "ass" and
> "subtitles" filter. Normally, you'd have to call ass_set_fonts() only
> exactly once to initialize all font things.
>
>
Indeed, that's why I wanted to call a function that did an update rather
than a re-initialization. If we agree that calling ass_set_fonts() is the
correct way to load those fonts, then moving it outside of init() seems
like a good option.


> > > > > +        ass_set_fonts(ass->renderer, NULL, NULL, 1, NULL, 1);
> > > > > +    }
> > > > > +
> > > > >      /* Open decoder */
> > > > >      dec_ctx = st->codec;
> > > > >      dec = avcodec_find_decoder(dec_ctx->codec_id);
> > > >
> > > > Otherwise it looks OK.
> > > >
> > >
> >
> > Thanks for the corrections, v2 coming soon.
> >
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


More information about the ffmpeg-devel mailing list