[FFmpeg-devel] [PATCH] lavc/avcodec.h: extend documentation for AVPicture API

[Stefano Sabatini]

On date Tuesday 2013-07-02 20:29:53 +0200, Michael Niedermayer encoded:
> On Thu, Jun 27, 2013 at 01:06:55PM +0200, Stefano Sabatini wrote:
> > ---
> >  libavcodec/avcodec.h | 52 ++++++++++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 46 insertions(+), 6 deletions(-)
> > 
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index 12a11aa..d0e206d 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -4286,15 +4286,18 @@ void av_resample_close(struct AVResampleContext *c);
> >   */
> >  
> >  /**
> > - * Allocate memory for a picture.  Call avpicture_free() to free it.
> > + * Allocate memory for a picture and fill picture fields with data
> > + * associated to the new allocated buffer.
> 
> maybe its just me but this sounds convoluted for the rather simply
> thing avpicture_alloc() does
> 
> allocate memory for the pixels of a picture and setup the AVPicture
> structure for it.

Changed.
> 
> >   *
> > - * @see avpicture_fill()
> > + * Call avpicture_free() to free it.
> >   *
> >   * @param picture the picture to be filled in
> >   * @param pix_fmt the format of the picture
> >   * @param width the width of the picture
> >   * @param height the height of the picture
> > - * @return zero if successful, a negative value if not
> > + * @return zero if successful, a negative error code otherwise
> > + *
> > + * @see av_image_alloc()
> >   */
> >  int avpicture_alloc(AVPicture *picture, enum AVPixelFormat pix_fmt, int width, int height);
> >  
> > @@ -4308,8 +4311,26 @@ int avpicture_alloc(AVPicture *picture, enum AVPixelFormat pix_fmt, int width, i
> >  void avpicture_free(AVPicture *picture);
> >  
> >  /**
> > - * Fill in the AVPicture fields, always assume a linesize alignment of
> > - * 1.
> > + * Setup the picture fields based on the specified image parameters
> > + * and the provided buffer.
> > + *
> > + * The picture fields are filled in by using the image data buffer
> > + * pointed to by ptr.
> 
> 
> > + * The image linesize alignment is always supposed
> > + * to be 1.
> 

> what is a image linesize alignment or rather how could the reader
> know.
> IMHO if you mean that the linesize is choosen so its the smallest
> possible value for the given width and pixel format then say that or
> say its not rounded up to some 2^x. But "alignment" can mean alot and
> in relation to pictures i dont think the intended meaning is the
> most obvious to everyone

Good observation, removed mention altogether, which shouldn't harm the
understanding of the API.

> 
> 
> > + *

> > + * If ptr is NULL, the function will fill the picture linesize array
> > + * and return the required size for the image buffer.
> 
> i assume it will do that too if ptr is not NULL ...
> I think rather (or too) the difference should be documented

No this is correct, I reworded it a bit to make it more clear.

[...]

Thanks for the review.
-- 
FFmpeg = Foolish Faithful Minimalistic Political Extensive Genius
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-lavc-avcodec.h-extend-documentation-for-AVPicture-AP.patch
Type: text/x-diff
Size: 4707 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130704/bf4ca10c/attachment.bin>