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

[Michael Niedermayer]

On Fri, Jun 13, 2014 at 03:03:17PM +0200, wm4 wrote:
> 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?

it is enabled for most decoders, just a few speed relevant ones
disable it and deal with the end differently and not all of these
need  0 padding.



> 
> 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.

IMHO the subtitle code should not make such assumtations, and these
are bugs that should be fixed

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

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- 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/20140613/29f39b66/attachment.asc>