[FFmpeg-devel] [PATCH] Unbreak av_malloc_max(0) API/ABI

["zhilizhao(赵志立)"]

> On Oct 15, 2020, at 1:14 AM, Joakim Tjernlund <joakim.tjernlund at infinera.com> wrote:
> 
> On Tue, 2020-10-13 at 20:33 +0200, Joakim Tjernlund wrote:
>> On Tue, 2020-10-13 at 20:11 +0200, Andreas Rheinhardt wrote:
>>> 
>>> Joakim Tjernlund:
>>>> On Tue, 2020-10-13 at 15:24 +0100, Derek Buitenhuis wrote:
>>>>> 
>>>>> On 13/10/2020 15:19, Joakim Tjernlund wrote:
>>>>>> For now just fixing av_malloc_max(0) will do, av_max_malloc2() etc. is beyond this patch.
>>>>> 
>>>>> As Ronald mentioned, if you're going to fix the ABI/API, you should actually
>>>>> fix every use of that ABI/API, not just the one you care about (0).
>>>>> 
>>>>> So max values of 1-31 should also be handled.
>>>> 
>>>> OK, how far do you want to take this, will this do?
>>>>     if (max < 32)
>>>>         max = INT_MAX;
>>>> 
>>> 
>>> The old behaviour was to effectively limit allocations to max - 32 in
>>> this case (near to SIZE_MAX), not to INT_MAX. And it does not restore
>>> default behaviour (the effective default limit was INT_MAX - 32).
>>> 
>>>> Complete API fix would need to revert the patch that caused the break to begin with and I don't think you want that ?
>>>> 
>>> Actually the old behaviour was against the documented API (or where do
>>> you see the 32 mentioned in the docs?).
>> 
>> The old behaviour in code, what the docs say isn't relevant to a failing app.
>> 
>>> 
>>> - Andreas
>>> 
>>> PS: My rationale for this patch was to match actual behaviour and
>>> documented behaviour and to make the full range of INT available for
>>> allocations if the maximum has not been overridden by av_max_alloc().
>>> Otherwise allocations of e.g. packets with sizes in the range (INT_MAX -
>>> 31 - AV_INPUT_BUFFER_PADDING_SIZE)..(INT_MAX -
>>> AV_INPUT_BUFFER_PADDING_SIZE) will fail.
>> 
>> Please suggest a patch to your liking, I am just trying to repair the breakage, I am not a ffmpeg dev.
>> 
> 
> Gentle ping

There is little benefit to keep such compatibility. And I wonder if there is anyone still use av_max_alloc(0).
More workaround means more burden for code maintenance. If there is no use case for av_max_alloc(0),
I think we can add an assert(max), but not change the value quietly.

> 
>  Jocke
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".