[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