[FFmpeg-devel] Structuring Commits in a Patchset

[Andreas Rheinhardt]

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.

- Andreas