[FFmpeg-devel] Structuring Commits in a Patchset

Soft Works softworkz at hotmail.com
Sun Sep 12 01:06:33 EEST 2021



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Andreas Rheinhardt
> Sent: Saturday, 11 September 2021 23:51
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] Structuring Commits in a Patchset
> 
> Soft Works:
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> >> Andreas Rheinhardt
> >> Sent: Saturday, 11 September 2021 23:02
> >> To: ffmpeg-devel at ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] Structuring Commits in a Patchset
> >>
> >> Soft Works:
> >>> Hi,
> >>>
> >>> I have a question about structuring my patchset .
> >>>
> >>> IIUC, commits inside a patchset must be separated by library.
> Now,
> >> Andreas
> >>> mentioned that every single commit in a patchset needs to pass
> >> FATE/compilation.
> >>>
> >>> What if both cannot be achieved at the same time, like when
> commit1
> >> for libA
> >>> breaks libB which can only be addressed in a separate subsequent
> >> commit2 for libB?
> >>>
> >> When the ABI is closed (as usual), then commit1 must not be
> >> committed;
> >> not even together with commit2, because users are allowed to use
> an
> >> old
> >> version of libB together with a new version of libA.
> >> (In such cases we typically add a new function if an old one is
> not
> >> sufficient anymore and leave the old function in place up until
> the
> >> next
> >> major bump if it is not public.)
> >>
> >> When the ABI is open (as now, but not indefinitely; we will
> probably
> >> make a new release in a month or so), you can just make the
> changes
> >> to
> >> both libraries at the same time. Notice that this does not allow
> >> changes
> >> to the public API; it is only the ABI side of things.
> >>
> >> - Andreas
> >>
> >> PS: Atomic commits are often automatically intra-library. But if a
> >> commit logically touches several libraries but not in any way that
> >> version conflicts between libraries might arise, then splitting
> along
> >> library lines makes no sense. E.g. look at
> >> 2934a4b9a5ee4825480180421e4679c02e6cbbe5 for an example.
> >
> > Thanks Andreas,
> >
> > I understood so far, except what to do when the public API is
> > affected as well..
> >
> > Specifically, I'm about to:
> >
> > - move AVSubtitleType and AVSubtitleRect from avcodec to avutil
> > - merge AVSubtitle (avcodec) into AVFrame (avutil)
> > - adjust everything else for this change:
> >   - avcodec
> >   - avfilter
> >   - ffmpeg, ffplay, ffprobe
> >
> > Should that be a single commit?
> >
> The first might be possible in the ABI-unstable period, but not when
> the
> ABI is stable.
> New fields can always be added at the end of AVFrame; but actually
> removing standalone AVSubtitles is a big API break that requires a
> deprecation period.
> If the API changes from your preceding commit break avfilter and the
> remaining internal use of AVSubtitle by avcodec (i.e. the subtitle
> decoders/encoders), then you a deprecation is definitely needed. If
> the
> ABI is stable, then it also mustn't break older avcodec/avfilter
> version
> (i.e. everything after the first release) when run with newer
> libavutil
> versions.
> 
> Generally speaking, you should not expect to be able to avoid having
> to
> deal with deprecations and with temporary glue code.

My current patchset (v4) doesn't create such impact. But when it is
desired to merge AVSubtitle into AVFrame, that would be a change
that is too fundamental to retain compatibility with some glue 
code.

I will submit an alternate version that implements that merge, 
then we can see where it would lead to.

softworkz


More information about the ffmpeg-devel mailing list