[FFmpeg-devel] [PATCH 4/9] avfilter/overlay_subs: Add overlay_subs filter

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Nicolas
> George
> Sent: Thursday, 19 August 2021 11:03
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 4/9] avfilter/overlay_subs: Add
> overlay_subs filter
> 
> Soft Works (12021-08-19):
> > I forgot to add [RFC PATCH]. I'm clearly not intending to have those
> > duplicate (and even unused) lines to be committed.
> >
> > It's just at a point where I'm wondering whether the idea that I have
> > described above (touching only relevant regions) could be implemented
> > easily or whether there might already exist something similar.
> 
> Ok.
> 
> > > But I can say immediately that, after a quick glance, I do not like your
> > > patch series at all, for a very simple reason: it is all over the place.
> > Remove the new filters and it's quickly getting more compact.
> 
> The number of lines of code is not the issue, the issue is the amount of
> existing code getting changed and that would need changing back to
> revert and implement something cleaner.
> 
> I will only accept two options, and I expect most other developers, if
> made to care about libavfilter, would agree:
> 
> - Quick-and-dirty code very closely constrained that can be removed
>   easily. (This is what I did with sub2video, I doubt it is possible in
>   libavfilter).
> 
> - Carefully designed code with always a clear path towards a clean
>   definite solution.
> 
> It seemed your proposal was a quick-and-dirty solution that touched a
> lot of code. With a second look, it is cleaner than that, but it does
> not have a clear path towards a really clean solution.

Yes. To be honest, it was focused on getting a quick result, but while
doing so, I realized that it is less involved than expected.
I had deliberately avoided to re-read the earlier conversation and
look at Clement's code to keep a free mind. But looking at Clement's
code afterwards revealed that what I ended up with is quite similar
to his earlier work.

> 
> > Where is my proposed code for libavfilter different from "proper"
> > subtitles support?
> 
> Where are the sshowinfo, ssplit, ssetpts, ssettb, etc. utility filters?
> 
> They are needed. But if your reaction is "this is ridiculous", you are
> 100% right. The short of it is that we cannot add a third media type
> without finding a way to have the neutral utility filters work for all
> media types at once without boilerplate duplication.

No, I don’t think this would be ridiculous. I just wasn't sure how far
I should go with the initial patchset. I also had an scopy filter
that I had removed before submitting because it wasn't needed from 
a proof-of-concept perspective.

> 
> That means negotiating the media type.
> 
> That requires refactoring the negotiation.

I'm not sure what you mean exactly, probably I should take a look 
at your code that you mentioned.

> 
> > I think at the libavfilter side, this is pretty how it will have
> > to be for "proper" implementation.
> > (anyway, it can't be much wrong as it is done just analog to audio and
> > video)
> 
> To begin with, we should properly discuss, *before impementing*:
> 
> - how a subtitle should be encoded into a frame, both bitmap and text;
> 
> - what kind of properties of the frame need to be negotiated.

That's one of the differences between mine and Clement's proposals:

We already have AVSubtitle which is understood by encoders and decoders,
so I see no reason to convert this back and forth to something different
just for carrying around by AVFrame.


> To begin with, we should properly discuss, *before impementing*:

I need a solution and the discussion last year ended up nowhere after
discussing some freaky details about which I (and probably most others)
don't care at all.
From that experience I wanted to avoid this to happen again.


> 
> > I think that the area that is remaining in a somewhat "dirty" state
> > is the heartbeat mechanism, which I have kept from sub2video.
> > As mentioned, I didn't want to make a step too big and rather keep
> > things working and compatible.
> 
> Sure, that is ok. But for a proper implementation, even if you do not
> implement it for years, you need to have a clear idea of how it will
> work in the final version. Again, that means discussing before
> implementing.

I don't think that my patchset is going a totally wrong way, even though
some things might need adjustment, but it is clearly just a step and 
not arrival at a finish line.

The primary motivation for submitting this right away as is, is to 
avoid this do be discussed to death once again and make a step that

- doesn't break anything
- allows things to work that haven't been possible before

..even when it's not yet the perfect solution in all aspects.


softworkz