[Ffmpeg-devel] [PATCH 01/31] Doxygenize existing comments

Panagiotis Issaris takis
Sun Mar 4 23:51:16 CET 2007


Hi,

Michael Niedermayer schreef:
> On Sun, Mar 04, 2007 at 11:11:26PM +0100, takis.issaris at uhasselt.be wrote:
>   
>> Doxygenize the existing comments in avformat.h. The contents of the comment was
>> left unaltered.
>>     
>
> patch ok
> comments unrelated to the doxygenization but rather related to the
> comments/doxy are below
>   

So, it is okay to apply the patch without the fixes to the content only 
adding the one comment I forgot to doxygenize?
And, of course fix the remarks below in follow-up patches?

>
> [...]
>   
>> -/* This is a hack - the packet memory allocation stuff is broken. The
>> -   packet is allocated if it was not really allocated */
>> +/**
>> + * @warning This is a hack - the packet memory allocation stuff is broken. The
>> + * packet is allocated if it was not really allocated
>> + */
>>     
>
> i wouldnt call t broken considering it works fairly well ...
>   
OK, I will remove that warning.

>   
>>  int av_dup_packet(AVPacket *pkt);
>>  
>>  /**
>> @@ -122,7 +124,7 @@ struct AVCodecTag;
>>  
>>  struct AVFormatContext;
>>  
>> -/* this structure contains the data a format has to probe a file */
>> +/** this structure contains the data a format has to probe a file */
>>  typedef struct AVProbeData {
>>     
>
> i think this should be rephrased ...
>   
Indeed, doesn't seem to make sense.

>
> [...]
>   
>>  typedef struct AVOutputFormat {
>>      const char *name;
>>      const char *long_name;
>>      const char *mime_type;
>> -    const char *extensions; /* comma separated extensions */
>> -    /* size of private data so that it can be allocated in the wrapper */
>> +    const char *extensions; /**< comma separated extensions */
>>     
>
> _filename_ extensions
>   
Will change this in "comma separated filename extensions" as suggested.

>
> [...]
>   
>>      int flags;
>> -    /* currently only used to set pixel format if not YUV420P */
>> +    /** currently only used to set pixel format if not YUV420P */
>>      int (*set_parameters)(struct AVFormatContext *, AVFormatParameters *);
>>     
>
> this is wrong i think
>   
Okay to just remove the entire comment?

> [...]
>   
>>      /* ffmpeg.c private use */
>> -    int stream_copy; /* if TRUE, just copy stream */
>> +    int stream_copy; /* *<if TRUE, just copy stream */
>>     
>
> TRUE? this isnt C++
>   
"if greater then zero, just copy stream */
or
"if >0, just copy stream */

> [...]
>
>   
>>      float quality;
>>      /* decoding: position of the first frame of the component, in
>>         AV_TIME_BASE fractional seconds. */
>>      int64_t start_time;
>>     
>
> not doxygenized?
>   
Oops forgot that one...
Okay to change that into the line below and apply it within this same patch?

     /** decoding: position of the first frame of the component, in
        AV_TIME_BASE fractional seconds. */


>
>   
>> -    /* decoding: duration of the stream, in AV_TIME_BASE fractional
>> +    /** decoding: duration of the stream, in AV_TIME_BASE fractional
>>         seconds. */
>>      int64_t duration;
>>     
>
> i do think that all stream specific time "things" are in AVStream.timebase
> units
>   
Ah, okay, I will fix this.


With friendly regards,
Takis




More information about the ffmpeg-devel mailing list