[FFmpeg-devel] [PATCH v18 01/19] avcodec, avutil: Move enum AVSubtitleType

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Monday, December 6, 2021 4:17 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v18 01/19] avcodec, avutil: Move enum
> AVSubtitleType
> 
> Quoting Soft Works (2021-12-03 11:25:08)
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Anton
> > > Khirnov
> > > Sent: Wednesday, December 1, 2021 2:35 PM
> > > To: ffmpeg-devel at ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [PATCH v18 01/19] avcodec, avutil: Move enum
> > > AVSubtitleType
> > >
> > > Quoting Soft Works (2021-11-29 20:47:52)
> > > > Signed-off-by: softworkz <softworkz at hotmail.com>
> > > > ---
> > > >  libavcodec/avcodec.h | 19 +--------------
> > > >  libavutil/subfmt.h   | 58 ++++++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 59 insertions(+), 18 deletions(-)
> > > >  create mode 100644 libavutil/subfmt.h
> > >
> > > This commit does way more than just moving the enum, it also
> > > deprecates existing enum values and adds new ones. That should be
> > > mentioned in the commit message and in doc/APIchanges.
> >
> > This is something I would need some help with. Which of my commits should
> > bump versions in which libs and where major or minor bump?
> 
> If you expect the whole set to be pushed at once, it is enough to have a
> separate patch at the end that bumps the versions of all the affected
> libraries and adds the necessary APIchanges entries.

I'm confused, previously I was told that patches should be atomic and I 
thought that APIchanges entries should be made in the same patchset.

Should this be a separate commit then, that only does APIchanges entries
and version bumps?

> > Here's what I _think_ (possibly wrong):
> > It's based on the V18 patchset
> >
> >
> > 01/19 avcodec,avutil: Move enum AVSubtitleType to avutil, add new and
> >
> > - major bump in avcodec because AVSubtitleType is removed
> > - major bump in avutil because AVSubtitleType is added
> 
> Enums exist only in the headers, they are not exported by the binaries
> in the way functions are. So moving an enum declaration from avcodec.h
> to lavu does not actually break compatibility as long as avcodec.h
> includes the new header (and thus provides the type to any legacy
> programs).
> 
> So this requires only a minor bump in libavutil for adding a new header
> and new enum values. A major bump would be required for changes that
> break existing callers, adding new declarations does not break
> anyything.

Sure. Will change that.

> >
> > 02/19 avutil/frame: Prepare AVFrame for subtitle handling
> >
> > - major bump in avutil because AVFrame struct is modified
> >   (not just adding to the end)
> 
> Why not just move your changes to the end?
> 
> You seem not to realize that a major bump is a big deal. We typically do
> it not more than once per years, and there was one already this spring.
> You should avoid API/ABI-breaking changes if possible, and it is very
> much possible here.

The position of the subtitle fields doesn't matter a lot. I thought
that the AVMediaType field would be too elementary to put it at the
end. But on the other hand: If that would be the only change causing 
a major bump, then it will surely make sense to add it to the end
(it could still be moved later when another major bump happens).

> > 03/19 avcodec/subtitles: Introduce new frame-based subtitle decoding
> >
> > - minor bump in avcodec. New APIs are added, existing API remains
> functional
> >
> > 04/19 avfilter/subtitles: Update vf_subtitles to use new decoding api
> > (no change)
> >
> > 05/19 avcodec,avutil: Move ass helper functions to avutil as avpriv_
> >
> > - major bump in avcodec and avutil
> >   no public APIs are changed but each lib depends on the other to be
> updated
> 
> A minor bump in libavutil. libavcodec must be used with libavutil from
> the same commit or newer.

OK, will change.

> > 06/19 avcodec/subtitles: Migrate subtitle encoders to frame-based API
> > (no change)
> > 07/19 fftools/play,probe: Adjust for subtitle changes
> > (no change)
> > 08/19 avfilter/subtitles: Add subtitles.c for subtitle frame allocation
> > (no change)
> >
> > 09/19 avfilter/avfilter: Handle subtitle frames
> >
> > - minor bump in avfilter due to additions to the end of AVFilter struct.
> 
> No bump - the changes are in a non-public part of that struct.

Understood.

So I make the changes, combine them into a single additional commit at the
end, right?

Thanks,
softworkz