[FFmpeg-devel] [PATCH v5 10/12] avfilter/textmod: Add textmod filter

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Andreas Rheinhardt
> Sent: Sunday, 12 September 2021 23:56
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v5 10/12] avfilter/textmod: Add
> textmod filter
> 
> Soft Works:
> > Signed-off-by: softworkz <softworkz at hotmail.com>
> > ---
> >  doc/filters.texi         |  64 +++++++
> >  libavfilter/Makefile     |   3 +
> >  libavfilter/allfilters.c |   1 +
> >  libavfilter/sf_textmod.c | 381
> +++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 449 insertions(+)
> >  create mode 100644 libavfilter/sf_textmod.c
> >

[..]

> > +
> > +static int filter_frame(AVFilterLink *inlink, AVFrame *src_frame)
> > +{
> > +    TextModContext *s = inlink->dst->priv;
> > +    AVFilterLink *outlink = inlink->dst->outputs[0];
> > +    int ret;
> > +    AVFrame *out;
> > +
> > +    outlink->format = inlink->format;
> > +
> > +    out = av_frame_clone(src_frame);
> 
> Why clone? You can just reuse src_frame as is.

Not then but now (see below)

> 
> > +    if (!out)
> > +        return AVERROR(ENOMEM);
> > +
> > +    for (unsigned i = 0; i < out->num_subtitle_rects; i++) {
> > +
> > +        AVSubtitleRect *rect = out->subtitle_rects[i];
> > +
> > +        if (rect->ass) {
> 
> Is there are actually a reason that num_subtitle_rects can't be taken
> at
> face value? Your query_formats callback after all signals that only
> ass
> subtitles are accepted.

What do you mean by "at face value" - 'fixed'?

> 
> > +            char *tmp = rect->ass;
> > +            rect->ass = process_dialog(s, rect->ass);
> 
> You may not be the sole owner of this AVSubtitleRect; after all,
> they are shared. Ergo you must not modify it. Is it possible that you
> believed that av_frame_clone() would make the frame writable? It does
> not. For non-subtitle frames, av_frame_make_writable() makes them
> writable; but it does not for subtitles, because you made
> av_frame_get_buffer2() a no-op for subtitles and so
> av_frame_make_writable() will temporarily increment the refcount and
> then decrement it again.

In the next update, subtitle frames will have the same behavior like
video and audio frames, with make_writable working properly.

Thanks,
softworkz