[FFmpeg-devel] [PATCH 2/8] png: Don't fail when a packet is larger than INT_MAX

[Michael Niedermayer]

On Mon, Mar 30, 2015 at 05:11:05PM +0200, wm4 wrote:
> On Mon, 30 Mar 2015 13:49:08 +0000
> Donny Yang <work at kota.moe> wrote:
> 
> > On 30 March 2015 at 02:48, Michael Niedermayer <michaelni at gmx.at> wrote:
> > 
> > > On Sun, Mar 29, 2015 at 11:05:41AM +0000, Donny Yang wrote:
> > > > Signed-off-by: Donny Yang <work at kota.moe>
> > > > ---
> > > >  libavcodec/pngenc.c | 2 --
> > > >  1 file changed, 2 deletions(-)
> > > >
> > > > diff --git a/libavcodec/pngenc.c b/libavcodec/pngenc.c
> > > > index 3697dbb..bd3aae5 100644
> > > > --- a/libavcodec/pngenc.c
> > > > +++ b/libavcodec/pngenc.c
> > > > @@ -373,8 +373,6 @@ static int encode_frame(AVCodecContext *avctx,
> > > AVPacket *pkt,
> > > >              enc_row_size +
> > > >              12 * (((int64_t)enc_row_size + IOBUF_SIZE - 1) /
> > > IOBUF_SIZE) // 12 * ceil(enc_row_size / IOBUF_SIZE)
> > > >          );
> > > > -    if (max_packet_size > INT_MAX)
> > > > -        return AVERROR(ENOMEM);
> > >
> > > the check is neccessary to prevent potential integer overflows
> > >
> > 
> > Doesn't ffmpeg support memory allocations of greater than 4 GiB?
> > I thought it did because the memory allocation functions either accept an
> > int64_t or size_t...
> 
> No.

That is false, the maximum allocation size is set by av_max_alloc()
its INT_MAX by default but can be increased.

The reason for it being INT_MAX by default is that it limits broken
media streams from allocating insane amounts of memory and by limiting
below the 32bit range it protects against integer overflows in
places that use int to store a size or index


> The codebase is so rotten,

The mailing lists and IRC channels exist to foster the development and
use of FFmpeg. Non-constructive or off-topic messages, along with
other abuses, are not welcome.


> libavutil/mem.c even limits memory
> allocations to INT_MAX.

this is misleading, it limits to a user settable variable that has a
default of INT_MAX


> In your specific case the problem is that
> AVPacket.size is an int.

yes and no.
yes, AVPacket.size could be changed to int64_t, that would need major
ABI version and soname bumps of all libs though. Its something that
could be done during the next such bump.

but that is missing a point, AVPackets of >4GiB are very unwieldy
and would be very akward to handle in applications

If we do add support for frames larger than 4gb samples, then maybe
a different system than storing all of it in a single array in memory
should be considered.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150330/334d1e44/attachment.asc>