[FFmpeg-devel] [PATCH v5 00/25] Subtitle Filtering 2022

[Soft Works]

> -----Original Message-----
> From: Nicolas George <george at nsup.org>
> Sent: Saturday, July 2, 2022 7:18 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Cc: Michael Niedermayer <michael at niedermayer.cc>; softworkz
> <softworkz at hotmail.com>; Andriy Gelman <andriy.gelman at gmail.com>;
> Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> Subject: Re: [FFmpeg-devel] [PATCH v5 00/25] Subtitle Filtering 2022
> 
> Paul B Mahol (12022-07-02):
> > Can this be properly finally be fully reviewed and accepted?
> 
> As long as the patch does not have a solution to have all the utility
> filters (setpts, trim, concat, etc.) working with subtitles too
> without
> more code duplication, the review is just "NAK".

The latest versions has added support for trim (strim).

I strongly disagree on the "code duplication" argument. AFAIU, this
is about having similar filters duplicated for each media type, 
e.g. trim, atrim and strim.

The duplication has always existed already between audio and video
filters. You could have unified and eliminated that duplication for
years, but you didn't.

Now that a third media type is being added, it is totally arbitrary
to ask for unification and de-duplication of the code. You didn't 
do it for 2 media types, so why should 3 be the magic number where 
this is suddenly a requirement? This is an unjustified request.

Even when I would follow that demand, it wouldn't be reasonable, 
because in that case, the patchset would dramatically widen its 
scope and start affecting audio and video as well - which is much 
too huge to get it tackled in a single patchset.

It has always been possible to de-duplicate between audio and video,
and in the future, it will always be possible to de-duplicate between
audio, video and subtitles.

It might make sense to do so at some point in time, but it is out
of the scope of this patchset.
And when this is the only objection, then I think that the patchset
is actually in really good shape for getting merged.

Thanks,
softworkz