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

[Michael Niedermayer]

On Thu, Jun 12, 2014 at 07:04:57PM +0200, wm4 wrote:
> On Thu, 12 Jun 2014 08:42:13 +0200
> Christophe Gisquet <christophe.gisquet at gmail.com> 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?
> > 
> > Regarding the value, I see it bumped to 8 to 64 and reverted in March
> > 2010 without much comment (reverting accidental commit?), bumped from
> > 8 to 16 in June 2011 (no API bump) for an equivalent reason to the
> > current bump.
> 
> API issues weren't taken so seriously in the past. Ensuring API and ABI
> stability is a relatively new thing.
> 

> I would argue that this is a major change, because the input padding is
> the size of data lavc will access outside of the buffer bounds. So if a
> program is compiled to pad only 16 bytes, and then lavc is updated,
> lavc will suddenly do out of bounds accesses.

There are 2 things here, or well 3 actually
A. The amount of padding that our libs add, libavcodec is one of them
B. The amount of padding that libavcodec needs / accesses
C. The amount of padding that libavfilter/libswscale needs

This commit increases the amount of padding that is added, this is
not an API nor ABI change.
arguably the code is unclear about this so ill rethink this and submit
a better patch


> 
> But judging from the commit message, the size was bumped to paint over
> another bug (existing code reading past the 16 bytes padding). That's
> an interesting way to fix bugs.

The issue is that input buffers to libavcodec which have 16
bytes padding are passed into libavfilter which requires more padding

If you want to make sure libavfilter and libswscale will
never read more than 16bytes over, this is welcome of course.
And i dont mind helping, but unless someone does make such changes
it does require more padding and not providing that padding isnt
going to make the code work better

also iam not sure how easy it is to avoid 32byte padding with AVX
which works in 32byte blocks though. Knowing that you can go over by
32bytes might simplify some current and future avx optimizations quite
a bit compared to the alternative

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

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- 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/20140612/1ff4b14e/attachment.asc>