[FFmpeg-devel] [PATCH v8 11/13] avfilter/stripstyles: Add stripstyles filter

[Andreas Rheinhardt]

Soft Works:
> - stripstyles {S -> S)
>   Remove all inline styles from subtitle events
> 
> Signed-off-by: softworkz <softworkz at hotmail.com>
> ---
>  libavfilter/Makefile         |   1 +
>  libavfilter/allfilters.c     |   1 +
>  libavfilter/sf_stripstyles.c | 211 +++++++++++++++++++++++++++++++++++
>  3 files changed, 213 insertions(+)
>  create mode 100644 libavfilter/sf_stripstyles.c
> 
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index e6fef97c08..309c404bf7 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -540,6 +540,7 @@ OBJS-$(CONFIG_NULLSINK_FILTER)               += vsink_nullsink.o
>  OBJS-$(CONFIG_CENSOR_FILTER)                 += sf_textmod.o
>  OBJS-$(CONFIG_SHOW_SPEAKER_FILTER)           += sf_textmod.o
>  OBJS-$(CONFIG_TEXTMOD_FILTER)                += sf_textmod.o
> +OBJS-$(CONFIG_STRIPSTYLES_FILTER)            += sf_stripstyles.o
>  
>  # multimedia filters
>  OBJS-$(CONFIG_ABITSCOPE_FILTER)              += avf_abitscope.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index fc72858d18..e2e4deea22 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -528,6 +528,7 @@ extern const AVFilter ff_avf_showwavespic;
>  extern const AVFilter ff_vaf_spectrumsynth;
>  extern const AVFilter ff_sf_censor;
>  extern const AVFilter ff_sf_show_speaker;
> +extern const AVFilter ff_sf_stripstyles;
>  extern const AVFilter ff_sf_textmod;
>  extern const AVFilter ff_svf_graphicsub2video;
>  extern const AVFilter ff_svf_textsub2video;
> diff --git a/libavfilter/sf_stripstyles.c b/libavfilter/sf_stripstyles.c
> new file mode 100644
> index 0000000000..15c00e73b1
> --- /dev/null
> +++ b/libavfilter/sf_stripstyles.c
> @@ -0,0 +1,211 @@
> +/*
> + * Copyright (c) 2021 softworkz
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * text subtitle filter which removes inline-styles from subtitles
> + */
> +
> +#include <libavcodec/ass.h>

This header and the API in it is not public.

> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/opt.h"
> +#include "avfilter.h"
> +#include "internal.h"
> +#include "libavcodec/avcodec.h"
> +#include "libavcodec/ass_split.h"
> +
> +typedef struct StripStylesContext {
> +    const AVClass *class;
> +    enum AVSubtitleType format;
> +    int remove_animated;
> +} StripStylesContext;
> +
> +typedef struct DialogContext {
> +    const AVClass *class;

An AVClass is for options and logging (and would actually need an
AVClass, which you didn't define); you are not using it for that, so
just remove this.

> +    const StripStylesContext* ss_ctx;
> +    AVBPrint buffer;
> +    int drawing_scale;
> +    int is_animated;
> +} DialogContext;
> +
> +static void dialog_text_cb(void *priv, const char *text, int len)
> +{
> +    DialogContext *s = priv;
> +    if (!s->drawing_scale && (!s->is_animated || !s->ss_ctx->remove_animated))
> +        av_bprint_append_data(&s->buffer, text, len);
> +}
> +
> +static void dialog_new_line_cb(void *priv, int forced)
> +{
> +    DialogContext *s = priv;
> +    if (!s->drawing_scale && !s->is_animated)
> +        av_bprint_append_data(&s->buffer, forced ? "\\N" : "\\n", 2);
> +}
> +
> +static void dialog_drawing_mode_cb(void *priv, int scale)
> +{
> +    DialogContext *s = priv;
> +    s->drawing_scale = scale;
> +}
> +
> +static void dialog_animate_cb(void *priv, int t1, int t2, int accel, char *style)
> +{
> +    DialogContext *s = priv;
> +    s->is_animated = 1;
> +}
> +
> +static void dialog_move_cb(void *priv, int x1, int y1, int x2, int y2, int t1, int t2)
> +{
> +    DialogContext *s = priv;
> +    if (t1 >= 0 || t2 >= 0)
> +        s->is_animated = 1;
> +}
> +
> +static const ASSCodesCallbacks dialog_callbacks = {
> +    .text             = dialog_text_cb,
> +    .new_line         = dialog_new_line_cb,
> +    .drawing_mode     = dialog_drawing_mode_cb,
> +    .animate          = dialog_animate_cb,
> +    .move             = dialog_move_cb,
> +};
> +
> +static int query_formats(AVFilterContext *ctx)
> +{
> +    AVFilterFormats *formats;
> +    AVFilterLink *inlink = ctx->inputs[0];
> +    AVFilterLink *outlink = ctx->outputs[0];
> +    static const enum AVSubtitleType subtitle_fmts[] = { AV_SUBTITLE_FMT_ASS, AV_SUBTITLE_FMT_NONE };
> +    int ret;
> +
> +    /* set input subtitle format */
> +    formats = ff_make_format_list(subtitle_fmts);
> +    if ((ret = ff_formats_ref(formats, &inlink->outcfg.formats)) < 0)
> +        return ret;
> +
> +    /* set output video format */
> +    if ((ret = ff_formats_ref(formats, &outlink->incfg.formats)) < 0)
> +        return ret;
> +
> +    return 0;
> +}
> +
> +static char *ass_get_line(int readorder, int layer, const char *style,
> +                        const char *speaker, const char *effect, const char *text)
> +{
> +    return av_asprintf("%d,%d,%s,%s,0,0,0,%s,%s",
> +                       readorder, layer, style ? style : "Default",
> +                       speaker ? speaker : "", effect, text);
> +}
> +
> +static char *process_dialog(StripStylesContext *s, const char *ass_line)
> +{
> +    ASSDialog *dialog = ff_ass_split_dialog(NULL, ass_line);
> +    char *result = NULL, *text;
> +    DialogContext dlg_ctx = { 0 };
> +
> +    if (!dialog)
> +        return NULL;
> +
> +    dlg_ctx.ss_ctx = s;
> +
> +    av_bprint_init(&dlg_ctx.buffer, 0, AV_BPRINT_SIZE_UNLIMITED);
> +
> +    ff_ass_split_override_codes(&dialog_callbacks, &dlg_ctx, dialog->text);

This function is in libavcodec and is not public; it is not available
when using shared linking. The same goes for all the other ff_ass_*
functions. Generally, the ff-prefix denotes something that is library
internal, but is not static, i.e. not confined to one translation unit.

> +
> +    av_bprint_finalize(&dlg_ctx.buffer, &text);
> +
> +    if (text && strlen(text) > 0)

The AVBPrint API actually keeps track of the length of the buffer for
you: dlg_ctx.buffer.len. And anyway, checking whether a string has
positive strlen can simply be done by "*text != '\0'" (or to "*text").
Notice that an AVBPrint can be truncated on allocation failure. This
needs to be checked (and often isn't).

> +        result = ass_get_line(dialog->readorder, dialog->layer, dialog->style, dialog->name, dialog->effect, text);
> +
> +    av_free(text);

This is not how the AVBPrint is supposed to be used; you are wasting the
small string-optimization.

> +    ff_ass_free_dialog(&dialog);
> +
> +    return result;
> +}
> +
> +static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
> +{
> +    StripStylesContext *s = inlink->dst->priv;
> +    AVFilterLink *outlink = inlink->dst->outputs[0];
> +
> +    outlink->format = inlink->format;
> +
> +    av_frame_make_writable(frame);
> +    if (!frame)
> +        return AVERROR(ENOMEM);

This is complete nonsense: C uses call-by-value, so
av_frame_make_writable just can't modify the pointer at all even if it
wanted. Instead it returns a negative value on error.
(And you are also leaking frame.)

> +
> +    for (unsigned i = 0; i < frame->num_subtitle_areas; i++) {
> +
> +        AVSubtitleArea *area = frame->subtitle_areas[i];
> +
> +        if (area->ass) {
> +            char *tmp = area->ass;
> +            area->ass = process_dialog(s, area->ass);
> +
> +            if (area->ass) {
> +                av_log(inlink->dst, AV_LOG_DEBUG, "original: %s\n", tmp);
> +                av_log(inlink->dst, AV_LOG_DEBUG, "stripped: %s\n", area->ass);
> +            }
> +            else
> +                area->ass = av_strdup("");

Unchecked allocation.

> +
> +            av_free(tmp);
> +        }
> +    }
> +
> +    return ff_filter_frame(outlink, frame);
> +}
> +
> +#define OFFSET(x) offsetof(StripStylesContext, x)
> +#define FLAGS (AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_FILTERING_PARAM)
> +
> +static const AVOption stripstyles_options[] = {
> +    { "remove_animated", "remove animated text (default: yes)",   OFFSET(remove_animated),  AV_OPT_TYPE_BOOL, {.i64 = 1 }, 0, 1, FLAGS, 0 },
> +    { NULL },
> +};
> +
> +AVFILTER_DEFINE_CLASS(stripstyles);
> +
> +static const AVFilterPad inputs[] = {
> +    {
> +        .name         = "default",
> +        .type         = AVMEDIA_TYPE_SUBTITLE,
> +        .filter_frame = filter_frame,
> +    },
> +};
> +
> +static const AVFilterPad outputs[] = {
> +    {
> +        .name          = "default",
> +        .type          = AVMEDIA_TYPE_SUBTITLE,
> +    },
> +};
> +
> +const AVFilter ff_sf_stripstyles = {
> +    .name          = "stripstyles",
> +    .description   = NULL_IF_CONFIG_SMALL("Strip subtitle inline styles"),
> +    .query_formats = query_formats,
> +    .priv_size     = sizeof(StripStylesContext),
> +    .priv_class    = &stripstyles_class,
> +    FILTER_INPUTS(inputs),
> +    FILTER_OUTPUTS(outputs),
> +};
> +
>