[FFmpeg-devel] [PATCH] avcodec: increase FF_INPUT_BUFFER_PADDING_SIZE to 32

[Andreas Cadhalpun]

On 21.07.2014 00:48, Michael Niedermayer wrote:
> On Sun, Jul 20, 2014 at 10:43:30PM +0200, Andreas Cadhalpun wrote:
>> On 12.06.2014 08:42, Christophe Gisquet wrote:
>>> Hi,
>>>
>>> 2014-06-11 21:29 GMT+02:00 Michael Niedermayer <michaelni at gmx.at>:
>>>> -#define FF_INPUT_BUFFER_PADDING_SIZE 16
>>>> +#define FF_INPUT_BUFFER_PADDING_SIZE 32
>>>
>>> So, following the discussion on how API users may be affected by this
>>> change, does that need an API bump?
>>
>> One effect of this change is that now mplayer2 fails to build,
>> because it uses this check:
>> #if MP_INPUT_BUFFER_PADDING_SIZE < FF_INPUT_BUFFER_PADDING_SIZE
>> #error MP_INPUT_BUFFER_PADDING_SIZE is too small!
>> #endif
>>
>> MP_INPUT_BUFFER_PADDING_SIZE is defined as 16. Increasing that to 32
>> allows building mplayer2 again.
>>
>> So I think an API bump wouldn't have been a bad idea.
>
> I think a soname bump would cause alot more work for everyone

That's for sure. I didn't mean a soversion bump for this change alone, 
but rather wait with this change until a soversion bump is necessary due 
to a larger API change.
It's just my expectation that if the API (i.e. major soversion) doesn't 
change, programs that built successfully with an older version will 
continue to build fine.
This change breaks that expectation.

> but lets assume we did bump soname and changed it to 32 after that
>
> what with the old API/ABI ? it needs 32 too in the exact same corner
> cases that the new API/ABI needs it. But all apps that are build
> against the old AND which use these corner cases are then buggy.
> that doesnt sound better to me

I don't know how severe these corner case bugs have been.
Would it have been a big problem to not fix them until the next 
soversion bump?

> IMHO there where just 2 options
> either one changes FF_INPUT_BUFFER_PADDING_SIZE to 32 or one doesnt
> use more than 16bytes ever. Bumping API is not a fix for this
> And iam trying to change functions where easily possible to require
> less padding, help there is sure welcome

Luckily mplayer2 can be trivially adjusted for this change, so I don't 
think it's necessary to revert this commit. But if such a change is 
necessary again in the future, I would be great if it was coupled to a 
soversion bump.
In any case mentioning such a change in APIChanges would be good.

Best regards,
Andreas