[FFmpeg-devel] [PATCH 1/2] avfilter/transform: Stop exporting internal functions

[Andreas Rheinhardt]

James Almer:
> On 2/24/2021 9:31 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 2/24/2021 11:22 AM, Andreas Rheinhardt wrote:
>>>> avfilter_transform, avfilter_(add|sub|mult)_matrix are not part of the
>>>> public API (transform.h is not a public header), yet they are currently
>>>> exported because of their name. This commit changes this:
>>>> avfilter_transform is renamed to ff_affine_transform; the other
>>>> functions are just removed as they have never been used at all.
>>>
>>> The symbols are exported and have been in four releases so far for this
>>> soname. If we plan on making a 4.4 release before the bump, it may be a
>>> good idea if we keep the symbols around for the sake of not affecting
>>> the ABI, so I'm inclined towards just wrapping them in a
>>> LIBAVFILTER_VERSION_MAJOR < 8 check like we do when we remove avpriv
>>> ones.
>>>
>>
>> And why? There was never a legitimate outside user of these functions.
> 
> Because it's still exported. But ok, since nobody seems to think it's
> worth bothering with, this patch should be ok as is.
> 
>> And removing avfilter_all_channel_layouts in
>> d5e5c6862bbc834d5a036a3fa053a7569d84f928 (which also didn't have a
>> legitimate outside user) didn't seem to break anything either.
> 
> That function and others are mentioned as added in APIChanges yet their
> removal is not. Sounds like it was handled poorly...
> 

The whole formats API was scheduled to be made private in
b74a1da49db5ebed51aceae6cacc2329288a92c1 in May 2012. It was removed in
Libav in 1961e46c15c23a041f8d8614a25388a3ee9eff63 (less than a month
after its deprecation...) which was merged into FFmpeg in
5916bc46581230c68c946c0b4733cce381eddcbd in June 2012. It was the merge
commit that removed avfilter_all_channel_layouts (which is something
that Libav never had). None of these three commits added any entry to
APIChanges for their removal which was handled poorly; but
d5e5c6862bbc834d5a036a3fa053a7569d84f928 was not (it would be wrong to
add an APIChanges entry for something that hasn't been public API for
years).

>> Btw: 88d80cb97528d52dac3178cf5393d6095eca6200 broke ABI for x64, because
>> older versions of libavformat exchange a PutBitContext with libavcodec
>> via avpriv_align_put_bits and avpriv_copy_bits. So we can't really make
>> a 4.4 release.
> 
> That commit could be reverted in the release/4.4 branch. But yeah, it
> should have not been committed until those two functions were removed,
> since they tie PutBitContext to the ABI.

That will break ABI for those people who use a libavformat version >=
88d80cb97528d52dac3178cf5393d6095eca6200 and older than the commits that
removed PutBitContext from the ABI. Granted, not a release, but
nevertheless.
(Of course this problem would not even exist if we disallowed using
libraries from different commits together.)

> A crappy situation overall, but it should not prevent us from making one
> last release before the bump. The amount of additions to the libraries
> is considerable, and the first release post bump will be missing a lot
> of old API, so adoption could take a bit, and something newer than 4.3
> should be readily available long before that.