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

Nicolas George george at nsup.org
Thu Aug 19 12:03:21 EEST 2021


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.

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

That means negotiating the media type.

That requires refactoring the negotiation.

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

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

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210819/bcf2f313/attachment.sig>


More information about the ffmpeg-devel mailing list