[FFmpeg-devel] [PATCH] avcodec: add AVCODEC_REQUIRED_INPUT_BUFFER_PADDING_SIZE, split FF_INPUT_BUFFER_PADDING_SIZE

[wm4]

On Fri, 13 Jun 2014 14:25:58 +0200
Michael Niedermayer <michaelni at gmx.at> wrote:

> On Fri, Jun 13, 2014 at 01:58:29PM +0200, wm4 wrote:
> > On Fri, 13 Jun 2014 13:47:41 +0200
> > Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> > 
> > > On 13.06.2014, at 12:41, wm4 <nfxjfg at googlemail.com> wrote:
> > > 
> > > > On Fri, 13 Jun 2014 03:32:59 +0200
> > > > Michael Niedermayer <michaelni at gmx.at> wrote:
> > > > 
> > > >> On Fri, Jun 13, 2014 at 02:49:01AM +0200, wm4 wrote:
> > > >> 
> > > >> AVCODEC_REQUIRED_INPUT_BUFFER_PADDING_SIZE represents the amount of
> > > >> padding that is required for input to libavcodec
> > > >> 
> > > >> FF_INPUT_BUFFER_PADDING_SIZE represents the amount that we pad.
> > > >> Using this we can pass the output from libavcodec to other libs like
> > > >> libavfilter in the case where (due to rawvideo) the input has been
> > > >> passed through
> > > >> 
> > > >> I hope above is understandable
> > > > 
> > > > Yes, but still confusing. At least it could be a global constant, maybe
> > > > defined in libavutil/mem.h? Actually, why does not just av_malloc add
> > > > the padding? It also ensures alignment, so why not. (It will waste some
> > > > memory, but you do that for alignment too, with every allocation, even
> > > > strings.)
> > > > 
> > > > Declaring that input buffers must be allocated with av_malloc because
> > > > ffmpeg is so damn "special" is probably easier to understand for API
> > > > users. For advanced users, enumerating the memory buffer requirements
> > > > somewhere else might be useful too.
> > > 
> > > That has serious implications for zero-copy implementations, which at the 4k resolution level at least are critical.
> > 
> > I'm not sure how you mean this. Maybe the fact that some users will have
> > to copy the input buffer to get the required padding (and to zero it)?
> 
> Do you know of a real world application to which this applies ?
> That is one that needs to copy due to the padding and would not need
> to copy if there was no padding requirement

No, only some cases of users forgetting the padding. In practice, the
padding will still be there, either as start of the next memory
allocation, or fragmentation due to page size, which reduces the chance
of hard crashes with incorrect API usage.

Also, it's of course pretty inconvenient for the user to fulfill the
memory buffer requirements. Especially if things only go subtly wrong
if you don't.

> Its important to know if there is a real world case where this matters
> because if not then this kind of is just some philosophical issue
> while otherwise its something that should be looked into, to see if
> it can be improved

It's mostly the question of having a clean API with no weird traps.
This in turn can increase application stability.

> 
> also there should be no need to zero if you force
> UNCHECKED_BITSTREAM_READER to 0

What are the downsides of this, could it be enabled by default?

Anyway, seeing how the subtitle code assumes that it can treat packet
data as null-terminated strings (simply because it assumes 0-padding is
guaranteed) made me realize it won't be so simple.

> 
> [...]