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

[Michael Niedermayer]

On Sun, Mar 17, 2013 at 04:14:28AM +0100, Michael Niedermayer wrote:
> 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

patch applied

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- 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/20130318/f3681cd3/attachment.asc>