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

Michael Niedermayer michaelni
Mon Feb 26 12:07:47 CET 2007


Hi

On Sun, Feb 25, 2007 at 11:43:56PM +0100, Dujardin Bernard wrote:
> Hi,
> Attached a patch :
> -add standard license header to libavutil/fifo.h
> -add doxygen comments to libavutil/fifo.h
> -remove doxygen comments in libavutil/fifo.c
> 
> Regards
> Bernard

> Index: fifo.c
> ===================================================================
> --- fifo.c	(revision 8129)
> +++ fifo.c	(working copy)
> @@ -19,6 +19,13 @@
>   * License along with FFmpeg; if not, write to the Free Software
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
> + 
> +/**
> + * @file fifo.c
> + * A very simple circular buffer FIFO implementation.
> + * @author Fabrice Bellard
> + * @author Roman Shaposhnik
> + */

about authorship
if you do add @author tags then id expect that you investigate who the authors
are, that is dont just copy and paste the copyright names

svn blame with some sed says
      8 diego
     38 glantau
     45 michael
     23 romansh

also this is just who added code and might be very different from who wrote
it, IMHO >70% of the code in here is from fabrice but thats just a guess
if you mess with author and copyright tags you will have to spend many hours
carefully investigating who the real authors are, that is reading svn log and
svn blame of the file in question, a simple reindent will randomize svn blame
so this is not so easy ...
people including myself are fairly sensitive to having their name added or
removed at the wrong place

[...]
>  #ifndef FIFO_H
>  #define FIFO_H
>  
> +/**
> + * Circular fifo buffer.
> + */
>  typedef struct AVFifoBuffer {
> -    uint8_t *buffer;
> -    uint8_t *rptr, *wptr, *end;
> +    uint8_t *buffer;    ///< Begin pointer.
> +    uint8_t *rptr;      ///< Read pointer.
> +    uint8_t *wptr;      ///< Write pointer.
> +    uint8_t *end;       ///< End pointer.

these comments are completely redundant they provide no new
information to the reader, for external API i dont mind some extra
redundant comments which have the advantage of being nicely shown with
doxygen but for internal structs which the user has no business with
its not


>  } AVFifoBuffer;
>  
> +/**
> + * Initialize a fifo *.
> + * @param *f fifo buffer.
> + * @param size of fifo.
> + * @return -1 if fail 0 otherwise

return <0 for failure >=0 otherwise and thats almost always the case
the API specification MUST NOT be unneccessarily dependant on the
current implementation


> + */
>  int av_fifo_init(AVFifoBuffer *f, int size);
> +
> +/**
> + * Free a fifo *.

frees a fifo


> + * @param *f fifo buffer.
> + */
>  void av_fifo_free(AVFifoBuffer *f);
> +
> +/**
> + * Get size of a fifo *.

gets the size of a fifo

similarely for the others

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

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- 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/20070226/842c8184/attachment.pgp>



More information about the ffmpeg-devel mailing list