[FFmpeg-devel] [PATCH] avutil/frame: document alignment and padding requirements

[Michael Niedermayer]

On Sun, Mar 17, 2013 at 03:57:27AM +0100, Clément Bœsch wrote:
> On Sun, Mar 17, 2013 at 03:37:47AM +0100, Michael Niedermayer wrote:
> > On Sun, Mar 17, 2013 at 03:30:05AM +0100, Clément Bœsch wrote:
> > > On Sun, Mar 17, 2013 at 03:23:19AM +0100, Michael Niedermayer wrote:
> > > > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > > > ---
> > > >  libavutil/frame.h |    8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > > 
> > > > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > > > index 8fc5814..2330840 100644
> > > > --- a/libavutil/frame.h
> > > > +++ b/libavutil/frame.h
> > > > @@ -78,6 +78,11 @@ typedef struct AVFrame {
> > > >      /**
> > > >       * pointer to the picture/channel planes.
> > > >       * This might be different from the first allocated byte
> > > > +     *
> > > > +     * Some decoders access areas outside 0,0 - width,height, please
> > > > +     * see avcodec_align_dimensions2(). Some filters can read upto 16 bytes
> > > 
> > > up to
> > > 
> > > Also, what are the filters? filters from libavfilter? Or this can refer to
> > > codec filters?
> > > 
> > > Isn't swscale outside libavfilter also doing such extra read?
> > 
> > yes, i had considered swscale a filter too.
> > 
> 
> What about postproc?

its a filter too

Ideally someone should make a list of filters and their exact
requirements but thats not so easy ...


> 
> > swscale explicitly added
> > 
> > 
> > > 
> > > On a side note, a macro would be best.
> > > 
> > > > +     * beyond the planes, if these filters are to be used, then 16 extra
> > > > +     * bytes must be allocated.
> > > >       */
> > > >      uint8_t *data[AV_NUM_DATA_POINTERS];
> > > >  
> > > > @@ -87,6 +92,9 @@ typedef struct AVFrame {
> > > >       *
> > > >       * For audio, only linesize[0] may be set. For planar audio, each channel
> > > >       * plane must be the same size.
> > > > +     *
> > > > +     * For video the linesizes should be multiplies of the CPUs alignment
> > > > +     * preferrance. This is 16 or 32 for modern desktop CPUs
> > > 
> > > preference, missing trailing '.'
> > 
> > fixed
> > 
> > 
> > > 
> > > You said "should"; this isn't a requirement, right?
> > 
> > It depends on what you do with the AVFrames, some code can handle it
> > fine, some will print warnings, some will be slower, some will likely
> > crash.
> > 
> 
> You may want to signal this, because "should" sounds too light given the
> risks.

fixed


> 
> Are the crashes considered a bug in this case?

IMHO, no. If code says it needs aligned data ...
If its undocumented then it would be a bug (can be documentation or
code)
Of course not being considered a bug doesnt mean a patch removing
a requirement would not be welcome


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130317/1de38c14/attachment.asc>