[Ffmpeg-devel] [PATCH] Move libavformat Doxygen and other comments to avformat.h

Michael Niedermayer michaelni
Sat Mar 3 03:06:00 CET 2007


Hi

On Thu, Mar 01, 2007 at 05:12:46PM +0100, Panagiotis Issaris wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi,
> 
> The attached patch moves the libavformat public API comments to
> avformat.h. The comments are unmodified.
> 
>  avformat.h |  308
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  utils.c    |  278 -------------------------------------------------------
>  2 files changed, 308 insertions(+), 278 deletions(-)
> 
> If preferred, I can also send the same patch as 36 small patches, each
> moving one function's comments. I chose to not do this right away, as
> sending 36 patches at once might be considered a bit annoying,
> especially in this case as the comments are unmodified.
> 
> At the kernel mailinglist it is quite common to send such patches as
> replies [*] to one patch with a subject line [0/36] f.e.. Would that be
> a good idea for this mailinglist too?
[...]
> +/**
> + * Print  nice hexa dump of a buffer
> + * @param f stream for output
> + * @param buf buffer
> + * @param size buffer size
> + */
>  void av_hex_dump(FILE *f, uint8_t *buf, int size);
> +
> +/**
> + * Print on 'f' a nice dump of a packet
> + * @param f stream for output
> + * @param pkt packet to dump
> + * @param dump_payload true if the payload must be displayed too
> + */
>  void av_pkt_dump(FILE *f, AVPacket *pkt, int dump_payload);

these should be changed to av_logish style, (FILE dependancy sucks ...)
thats of course seperate and independant of this patch


[...]
> +/**
> + * Return the next frame of a stream.
> + *
> + * The returned packet is valid
> + * until the next av_read_frame() or until av_close_input_file() and
> + * must be freed with av_free_packet. For video, the packet contains
> + * exactly one frame. For audio, it contains an integer number of
> + * frames if each frame has a known fixed size (e.g. PCM or ADPCM
> + * data). If the audio frames have a variable size (e.g. MPEG audio),
> + * then it contains one frame.
> + *
> + * pkt->pts, pkt->dts and pkt->duration are always set to correct
> + * values in AV_TIME_BASE unit (and guessed if the format cannot

this is wrong, they are in AVStream.timebase units

[...]
> +/**
> + * Converts frame rate from string to a fraction.
> + *
> + * First we try to get an exact integer or fractional frame rate.
> + * If this fails we convert the frame rate to a double and return
> + * an approximate fraction using the DEFAULT_FRAME_RATE_BASE.

this is wrong, furthermore i would even say we shouldnt document how its
done exactly as this can change (it did already since this was written)


patch looks ok besides these, apply it (and then in a seperate commit
fix the 2 comments above that keeps svn log easy to review as the move
can be ignored ...)

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- 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/20070303/66dac516/attachment.pgp>



More information about the ffmpeg-devel mailing list