[Ffmpeg-devel] [RFC][PATCH]Doxygenize libavutil/fifo.h

Michael Niedermayer michaelni
Tue Feb 27 22:27:35 CET 2007


Hi

On Tue, Feb 27, 2007 at 05:33:53PM +0100, Dujardin Bernard wrote:
> Dujardin Bernard a ?crit :
> >Hi,
> >
> >I submit ;-)   attached patch which take account of your remarks.
> >
> I send this to take account the recent revision 8149.
> 
> Regards
> Bernard

> Index: fifo.h
> ===================================================================
> --- fifo.h	(revision 8149)
> +++ fifo.h	(working copy)
> @@ -16,6 +16,11 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
>  
> +/**
> + * @file fifo.h
> + * A very simple circular buffer FIFO implementation.
> + */

as already said remove the word "implementation" it is not appropriate
for the header


> + 
>  #ifndef FIFO_H
>  #define FIFO_H
>  
> @@ -24,15 +29,74 @@
>      uint8_t *rptr, *wptr, *end;
>  } AVFifoBuffer;
>  
> +/**
> + * Initializes a FIFO *.

initalizes an AVFifoBuffer
also the function does not initalize a pointer to a fifo but the fifo struct
itself so the * is not appropriate


> + * @param *f fifo buffer

@param f the AVFifoBuffer to initalize


> + * @param size of fifo
> + * @return <0 for failure >=0 otherwise
> + */
>  int av_fifo_init(AVFifoBuffer *f, int size);
> +
> +/**
> + * Frees a FIFO *.

frees an AVFifoBuffer


> + * @param *f fifo buffer

@param f the AVFifoBuffer to free


> + */
>  void av_fifo_free(AVFifoBuffer *f);
> +
> +/**
> + * Gets the size of a FIFO *.

what is the size of a fifo? this is unlear, if you dont know what a function
does then dont add a comment to it but leave it to someone else to do who does
goal is not to have as many comments as possible but to provide helpfull
information to the reader

correct here is
gets the amount of data in bytes in the fifo, that is the amount
you could read from it


> + * @param *f fifo buffer
> + * @return size
> + */
>  int av_fifo_size(AVFifoBuffer *f);
> +
> +/**
> + * Reads the data from the FIFO *.
> + * @param *f fifo buffer
> + * @param *buf data destination
> + * @param buf_size of data

@param buf_size number of bytes to read


> + * @return -1 if not enough data

ive already said that implementation details like -1 return are unaccptable
<0 is error >= 0 is no error
the doxygen comments in the header are a API specification they must be
written carefully, errors are totally unacceptable its much better if
something is undocumented


> + */
>  int av_fifo_read(AVFifoBuffer *f, uint8_t *buf, int buf_size);
> +
> +/**
> + * Reads the data from the FIFO *.

again dont write half correct comments, the comment is the same as for
the last but the function does not do the same

it rather feeds data from the fifo to a user supplied callback while
the previous reads data into a buffer


> + * @param *f fifo buffer
> + * @param buf_size data size
> + * @param *func generic read function
> + * @param *dest data destination

> + * @return -1 if not enough data

implementation detail


> + */
>  int av_fifo_generic_read(AVFifoBuffer *f, int buf_size, void (*func)(void*, void*, int), void* dest);
> +
> +/**
> + * Writes the data in the FIFO *.

"in" is wrong use "into"


> + * @param *f fifo buffer
> + * @param *buf data source
> + * @param size of data
> + */
>  void av_fifo_write(AVFifoBuffer *f, const uint8_t *buf, int size);
> +
> +/**
> + * Resizes the FIFO *.
> + * @param *f fifo buffer
> + * @param size
> + */
>  void av_fifo_realloc(AVFifoBuffer *f, unsigned int size);
> +
> +/**
> + * Discards the data from the FIFO *.

reads and descards the specified amount of data from the AVFifoBuffer


> + * @param *f fifo buffer
> + * @param size of data
> + */
>  void av_fifo_drain(AVFifoBuffer *f, int size);
>  
> +/**
> + * Returns a pointer with circular offset from FIFO's read pointer.
> + * @param *f fifo buffer
> + * @param offs offset
> + * @return ptr=rptr+offs if rptr+offs<end else rptr+offs -(end-begin)
> + */
>  static inline uint8_t av_fifo_peek(AVFifoBuffer *f, int offs)

this function should be removed as it exposes implemenbtation details
and should thus not be documented


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

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070227/f90d912c/attachment.pgp>



More information about the ffmpeg-devel mailing list