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

Soft Works softworkz at hotmail.com
Wed Sep 22 06:16:32 EEST 2021



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Andreas
> Rheinhardt
> Sent: Wednesday, 22 September 2021 04:48
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v8 11/13] avfilter/stripstyles: Add
> stripstyles filter
> 
> Soft Works:
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Andreas
> >> Rheinhardt
> >> Sent: Wednesday, 22 September 2021 03:58
> >> To: ffmpeg-devel at ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH v8 11/13] avfilter/stripstyles: Add
> >> stripstyles filter
> >>
> >> Soft Works:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Andreas
> >>>> Rheinhardt
> >>>> Sent: Wednesday, 22 September 2021 03:06
> >>>> To: ffmpeg-devel at ffmpeg.org
> >>>> Subject: Re: [FFmpeg-devel] [PATCH v8 11/13] avfilter/stripstyles: Add
> >>>> stripstyles filter
> >>>>
> >>>> 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.
> >>>
> >>> I'm not sure IIRC, but aren't there other cases where the ffmpeg libs
> >>> are accessing non-public symbols from each other?
> >>>
> >>
> >> Yes, there are via avpriv functions. Yet this is strongly discouraged,
> >> so the bar for new ones is pretty high.
> >>
> >>>
> >>>>> +
> >>>>> +#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.
> >>>
> >>> No I can't, because this is being passed to the
> >>> ff_ass_split_override_codes which expects an AVClass at the first
> >>> position of the struct.
> >>>
> >>>
> >>
> >> Why? The docs don't say so. And the opaque priv pointer is never used
> >> for logging. (Or did I miss something?)
> >
> > It crashed at some point, I don't remember where exactly, but the first
> > member of the struct got overwritten and invalidated. Then I realized that
> > it was assumed to have an AVClass.
> >
> 
> This surprises me. Are you sure that you did not forget the AVClass from
> your private filter context (in this case, StripStylesContext)?

That's something that happened as well, but at an earlier time and
with a different filter..

I think I had some logging in the callback functions (ASSCodesCallbacks)
before, probably that's how it happened to crash.

> 
> >>>>> +
> >>>>> +    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.
> >>>
> >>> I thought this could be ensured by having something like this in
> >>> configure:
> >>>
> >>> overlay_textsubs_filter_deps="avcodec libass"
> >>>
> >>
> >> This won't work (as it won't make libavcodec export these functions).
> >>
> >>>
> >>> If not, what would you suggest. Duplicate the ass code in avfilter,
> >>> move it to avutil and make it public, or any other idea?
> >>>
> >>
> >> Making it public? You know that your 10/13 would not be possible if this
> >> were public? (And it would also not work if it were avpriv; at least not
> >> without some versioning uglyness.)
> >> I don't like the idea of duplicating the code. The only vague suggestion
> >> that I have is that this might be more suitable for a bitstream filter.
> >
> > The ass subtitle format is used as the internal text subtitle format,
> > which means that those ass manipulation function are required to be
> > accessible by many other text subtitle filter in some way.
> > I'm using them in more filters already..
> >
> 
> Then we have a problem. And I don't have a solution.

Function prefixes per preprocessor defs to compile the same two source files
into both libs?

Moving into avutil would appear to make more sense to me. Anyway, it's exactly 
that: utility functions. (it's not the ass encoder or decoder)

softworkz






More information about the ffmpeg-devel mailing list