[FFmpeg-devel] [RFC] properly document av_crc

Reimar Döffinger Reimar.Doeffinger
Sun Aug 19 14:30:09 CEST 2007


Hello,
On Sun, Aug 19, 2007 at 02:10:06PM +0200, Michael Niedermayer wrote:
> On Sun, Aug 19, 2007 at 01:20:45PM +0200, Reimar D?ffinger wrote:
> > currently the crc functions in libavutil IMO have a very peculiar API
> > that in addition is also undocumented.
> > Attached patch documents it as good as I could come up with.
> > Though since it is not yet part of the public API I think it might be
> > worth changing it, e.g. make av_crc do the necessary bswaps...
> 
> no, iam against this
> the API is the way it is because its the most efficient
> the shift+bswap you propose would be unneeded in nearly all cases
> for checking the crc during reading its a matter of crc!=0 the
> order doesnt matter
> for writing its a matter of little vs. big endian writing functions
> and if multiple blocks would be handled your suggestion would have
> to do the shift bswap twice thus slowing the code down for small
> blocks

Yes, I am aware of that. I was only thinking about it because during
grepping for uses I mostly saw bswap_16(av_crc(... constructs which I
found a bit ugly.

> [...]
> > Index: libavutil/crc.c
> > ===================================================================
> > --- libavutil/crc.c	(revision 10141)
> > +++ libavutil/crc.c	(working copy)
> > @@ -37,6 +37,14 @@
> >   * Inits a crc table.
> >   * @param ctx must be an array of sizeof(AVCRC)*257 or sizeof(AVCRC)*1024
> >   * @param cts_size size of ctx in bytes
> > + * @param le if 1, lowest bit represents coefficient for highest exponent
> > + *           of corresponding polynomial (both for poly and actual CRC).
> 
> > + *           If 0, you must swap the crc parameter and the result of av_crc
> > + *           (can be simplified in most cases to e.g. bswap16):
> > + *           bswap_32(crc << (32-bits))
> 
> its not really "must" its only needed if you need the crc in a non "bswaped"
> format

Is
"you must swap the crc parameter and the result of av_crc if you need
the standard representation"
better? I do understand when it is required to do and when not, but I
don't know how to express it without pages of text ;-).

> [...]
> > @@ -70,6 +78,13 @@
> >      return 0;
> >  }
> >  
> > +/**
> > + * Calculate the CRC of a block
> > + * @param crc CRC of previous blocks if any or initial value for CRC.
> > + * @return CRC updated with the data from the given block
> > + *
> > + * Please read the explanation of the le parameter to av_crc_init
> 
> i think theres a @see tag or so for this

I can do '@see av_crc_init() "le" parameter', but I have no idea what
doxygen will make out of it, there is no documentation how to link to a
specific function parameter, only to functions...

Greetings,
Reimar D?ffinger




More information about the ffmpeg-devel mailing list