[FFmpeg-devel] [PATCH 1/4] swr: convert resample_common/linear_int16_mmx2/sse2 to yasm.

[Andreas Cadhalpun]

On 12.07.2014 20:34, Timothy Gu wrote:
> On Jul 12, 2014 11:15 AM, "Andreas Cadhalpun" <
> andreas.cadhalpun at googlemail.com> wrote:
>>
>> On 12.07.2014 19:37, Michael Niedermayer wrote:
>>>
>>> On Sat, Jul 12, 2014 at 06:58:49PM +0200, Andreas Cadhalpun wrote:
>>>>
>>>> On 12.07.2014 18:46, Michael Niedermayer wrote:
>>>>>
>>>>> On Sat, Jul 12, 2014 at 05:35:12PM +0200, Hendrik Leppkes wrote:
>>>>>>
>>>>>> On Sat, Jul 12, 2014 at 4:45 PM, Andreas Cadhalpun
>>>>>> <andreas.cadhalpun at googlemail.com> wrote:
>>>>>>>
>>>>>>> On 12.07.2014 15:40, Ronald S. Bultje wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On Sat, Jul 12, 2014 at 9:35 AM, Andreas Cadhalpun <
>>>>>>>> andreas.cadhalpun at googlemail.com> wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I'm a bit puzzled by this commit.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 30.06.2014 01:19, James Almer wrote:
>>>>>>>>>
>>>>>>>>>> From: Ronald S. Bultje <rsbultje at gmail.com>
>>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>>      libswresample/resample_template.c    |  23 +---
>>>>>>>>>>      libswresample/x86/resample.asm       | 225
>>>>>>>>>> ++++++++++++++++++++++++++---------
>>>>>>>>>>      libswresample/x86/resample_mmx.h     | 110 -----------------
>>>>>>>>>>      libswresample/x86/resample_x86_dsp.c |  44 +++----
>>>>>>>>>>      4 files changed, 187 insertions(+), 215 deletions(-)
>>>>>>>>>>
>>>>>>>>> [...]
>>>>>>>>>
>>>>>>>>>     diff --git a/libswresample/x86/resample_mmx.h
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> b/libswresample/x86/resample_mmx.h
>>>>>>>>>> index 94237b0..b0ea496 100644
>>>>>>>>>> --- a/libswresample/x86/resample_mmx.h
>>>>>>>>>> +++ b/libswresample/x86/resample_mmx.h
>>>>>>>>>> @@ -22,116 +22,6 @@
>>>>>>>>>>      #include "libavutil/cpu.h"
>>>>>>>>>>      #include "libswresample/swresample_internal.h"
>>>>>>>>>>
>>>>>>>>>> -DECLARE_ALIGNED(16, const uint64_t,
> ff_resample_int16_rounder)[2]    =
>>>>>>>>>> {
>>>>>>>>>> 0x0000000000004000ULL, 0x0000000000000000ULL};
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This removes the symbol ff_resample_int16_rounder from
> libswresample,
>>>>>>>>> which is a backwards incompatible ABI change.
>>>>>>>>>
>>>>>>>>> So can someone explain to me, why this hasn't been accompanied by
> a major
>>>>>>>>> SOVERSION bump of libswresample?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> All ff_* symbols are internal.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Then shouldn't all of them be declared with something like
> '__attribute__
>>>>>>> ((visibility ("hidden")))'?
>>>>>>>
>>>>>>
>>>>>> This is usually handled by a visibility file (.v), but for some reason
>>>>>> the swresample file doesn't hide ff_* symbols, but that doesn't really
>>>>>> change the fact that ff_* (and avpriv_*) are not part of the public
>>>>>> API/ABI and as such any changes or removals do not require a SONAME
>>>>>> bump.
>>>>>>
>>>>>
>>>>>> For the reason why they are not hidden, maybe Michael can comment.
>>>>>
>>>>>
>>>>> i assume this was just a copy and paste error, removed ff_* from
>>>>> swr
>>>>
>>>>
>>>> So should ff_* also be removed from libavutil.v?
>>>
>>>
>>> yes, but there is some chance that it might break some app so ill
>>> wait till after the release
>>
>>
>> Yes, for example this breaks libavfilter:
>> undefined reference to `ff_opencl_set_parameter'
>
> This means that the function should probably be avpriv_.
>
>>
>>
>>>> And what about avpriv_* in libavcodec, libavformat and libavutil?
>>>
>>>
>>> avpriv is shared between the libs so it has to be exported by each
>>> lib
>>
>>
>> So avpriv_* symbols shouldn't be removed without SONAME bump (in contrast
> to what Hendrik wrote)?
>
> What Hendrik wrote was about ff_, which *can* be removed without MAJOR
> bump. You are right about avpriv_.

OK, so either of us misunderstood what Hendrik wrote:
"[...] but that doesn't really change the fact that ff_* (and avpriv_*) 
are not part of the public API/ABI and as such any changes or removals 
do not require a SONAME bump."

> So, ff_ = can only be used within this library (in theory) and can be
> removed without major bump. It should not be exported or be used by users.
>
> avpriv_ = can only be used by FFmpeg libraries and cannot be removed
> without major bump. It should be exported, but should not be used by users.
>
> Other av* = can be used by everyone, and are exported and have proper
> prototypes or declarations in headers.

That makes sense.

> Right now, some libraries export some ff_ symbols as hacks and should be
> fixed by either converting them to avpriv_ or av_. libavutil exports all
> ff_ symbols until the next bump.

Yeah, I noticed the comment in libavcodec.v.

Best regards,
Andreas