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

[Ronald S. Bultje]

Hi,

On Tue, Oct 13, 2020 at 6:30 AM Joakim Tjernlund <
joakim.tjernlund at infinera.com> wrote:

> From https://bugs.chromium.org/p/chromium/issues/detail?id=1095962
> ----------------------------
> This seems to be caused by the custom handling of "av_max_alloc(0)" in
> Chromium's ffmpeg fork to mean unlimited (added in [1]).
>
> Upstream ffmpeg doesn't treat 0 as a special value; versions before 4.3
> seemingly worked
> because 32 was subtracted from max_alloc_size (set to 0 by Chromium)
> resulting in an
> integer underflow, making the effective limit be SIZE_MAX - 31.
>
> Now that the above underflow doesn't happen, the tab just crashes. The
> upstream change
> for no longer subtracting 32 from max_alloc_size was included in ffmpeg
> 4.3. [2]
>
> [1]
> https://chromium-review.googlesource.com/c/chromium/third_party/ffmpeg/+/73563
> [2] https://github.com/FFmpeg/FFmpeg/commit/731c77589841
> ---------------------------
>
> Restore av_malloc_max(0) to MAX_INT fixing MS Teams, Discord older
> chromium etc.
>
> Signed-off-by: Joakim Tjernlund <joakim.tjernlund at infinera.com>
> ---
>  libavutil/mem.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavutil/mem.c b/libavutil/mem.c
> index cfb6d8a..52c2b1b 100644
> --- a/libavutil/mem.c
> +++ b/libavutil/mem.c
> @@ -71,6 +71,8 @@ void  free(void *ptr);
>  static size_t max_alloc_size= INT_MAX;
>
>  void av_max_alloc(size_t max){
> +    if (!max)
> +        max = INT_MAX; /* be compatible to older(< 4.3) versions */
>      max_alloc_size = max;
>  }
>
> --
> 2.26.2


I think this is the wrong "unfix". The previous inversion of inverted
behaviour (b/c of the integer underflow) in the range [0,31] is now
maintained for [1,31], but not for 0.

I think that we can't "silently" change behaviour for functions like this.
As ugly as it is, the classic way of adding a new function
(av_max_malloc2()) and deprecating the old one is much better, because
people have to explicitly agree to the new function by updating their API
usage.

Just MHO.

Ronald