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

[Facundo Gaich]

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)

> 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?

> > > +        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