[FFmpeg-devel] [PATCH v5 00/12] Subtitle Filtering

[Hendrik Leppkes]

On Wed, Sep 15, 2021 at 1:21 AM Soft Works <softworkz at hotmail.com> wrote:
>
> Hendrik,
>
> I can't avoid to note that your reply pattern is a bit similar to
> what I've seen before here: long texts trying to point out one's
> authority, naming rejection conditions, trying to discourage instead
> of encourage and finally telling I would need to follow advice
> and guidance.
> Yet, each time I'm asking for advice and guidance - those are
> remaining silent that were telling me to follow.

Or perhaps some people have a day job, a life and other obligations
that prevent them from spending time on FFmpeg every day, especially
outside the weekend.
But no, that can't be it, surely we are all just evil. /s

>
> There's no point though, in trying to talk this into something
> more difficult as it is and invent any dubious blocking
> pre-conditions.

Nothing here is being invented. You are trying to push a major API
change to the core functionality of our libraries, these rules on how
to do API changes on that level (or any level, really) have been in
place and followed by everyone for years and years.
If you don't believe that, feel free to check how any other major API
change was handled in the past. Because the overall pattern is always
the same.

Do you think I'm the only one thinking that way, only because I spoke
up? The set certainly wouldn't have been applied if I hadn't said
anything, it would've just sat there. Maybe someone else might've come
forward eventually.

As for the actual subject:
- Part1, add subtitles to AVFrame in avutil. You can move
enums/structs to avutil as needed (eg AVSubtitleRect or whatever), as
long as they are still available the same way (eg. avcodec.h should
include any new avutil header with them, so user code works without
changes), but moving functions is a more tricky business, so we
usually make new functions and cleanup their naming convention in the
process, but I don't think any functions are involved here right now.

To dive a bit deeper into this, redundant fields should be removed,
actual subtitle data should be refcounted (with AVBuffers in
AVFrame->buf), all frame functionality need to properly account for
the new data type.

- Part2, avcodec support for subtitles in AVFrames. Not that long ago
we added a generic decoding API,
avcodec_send_packet/avcodec_receive_frame and encoding API
avcodec_send_frame/avcodec_receive_packet. Those are
mediatype-agnostic, so they can also handle subtitles. No need for a
new API, just new internal plumbing. The important part here is that
the old API must remain untouched and functional, which means the
easiest way for the time being is adding a compatibility shim that
uses the old internal API and then converts to an AVFrame (or
vice-versa for encoding). This can get inverted later, eg. all
de/encoders updated to a new internal API, and the old external API
being run through a shim, but I would recommend to actually do this
later, as it reduces the friction.

- Part3, avfilter support for subtitles in AVFrames. At this point we
have a defined structure to store subtitles in AVFrames, and actual
code that can generate or consume them. When approaching this, the
same rules apply as before, existing subtitle functionality, as crude
as it may be, has to remain functional as exposed to the user.
Internal under-the-hood changes are fine, as long as the user does not
notice.

I'm not involved in internal avfilter design, so at this point you'll
have to get Nicolas on board.

- Part4, update ffmpeg.c etc to make use of these new features

All these parts should be made up of as small incremental atomic
changes as possible, to ease reviewing and bisecting for issues later.
In every case the old subtitle functionality has to remain functional
as exposed to the user, once the new one is in place it can get
deprecated and removed in ~2 years time. Sometimes that means
developing something parallel, sometimes keeping the old one alive
with a compatibility shim, or basing the new one on a shim for now,
which can get inverted later.

Hope some of that helps.

- Hendrik