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

[Dujardin Bernard]

Michael Niedermayer a ?crit :
> Hi
>   
>> 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
>   
Of course, It's not in my mind to offend real authors.
 I've removed the author tag in fifo.h and fifo.c
and let's you write real authors
> [...]
>   
>>  #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
>
>   
Lets the Circular fifo buffer comment ?
>   
>>  } 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
>
>   
Applied
>   
>> + * @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
>   
Prefered 'Reads the datas'  or 'Reads the data' and similarely for the 
others ?

Wait for your answer and commit another patch