[FFmpeg-devel] [PATCH] Improve documentation for libavutil/base64.h

Stefano Sabatini stefano.sabatini-lala
Sun Jan 25 02:04:52 CET 2009


On date Sunday 2009-01-25 00:27:23 +0100, Michael Niedermayer encoded:
> On Sat, Jan 24, 2009 at 09:26:43PM +0100, Stefano Sabatini wrote:
> > Hi,
> > 
> > in the patch I'm also changing the parameter names, I think they're
> > more meaningful now, also I'm applying the distinction between size
> > (size in bytes of a data buffer) and lenght, intended as the number of
> > characters in a string *but* the last one, which looks consistent with
> > the terminology of the C language (sizeof(...), strlen(...)).
> > 
> > Regards.
> > -- 
> > FFmpeg = Fast and Fantastic Minimal Philosofic Entertaining God
> 
> > Index: libavutil/base64.h
> > ===================================================================
> > --- libavutil/base64.h	(revision 16736)
> > +++ libavutil/base64.h	(working copy)
> > @@ -25,16 +25,26 @@
> >  #include <stdint.h>
> >  
> >  /**
> > - * decodes base64
> > - * param order as strncpy()
> > + * Decode the base-64 string in \p src and put the decoded data in \p
> > + * dst.
> 
> av_base64_decode() decodes the base-64 string in ...
> not
> av_base64_decode() decode the base-64 string in ...

I agree, and this had been already discussed mmhh.. almost two years
ago and we defined to stick to the Javadoc recommendations, which in
this specific case recommends the use of the third person forms, but
most libav* codes uses the impersonal form.

Diego could you say which form do you prefer once and for all?  Also
setting it in the patch rules may help to keep consistency and avoid
boring discussions.
  
> > + *
> > + * @param dst_size size in bytes of the \p dst buffer, it has to be at
> > + * least 3/4 of the length of \p src
> > + * @return the number of bytes written, or a negative value in case of
> > + * error
> >   */
> > -int av_base64_decode(uint8_t * out, const char *in, int out_length);
> > +int av_base64_decode(uint8_t *dst, const char *src, int dst_size);
> >  
> >  /**
> > - * encodes base64
> > - * @param src data, not a string
> > - * @param buf output string
> > + * Encode in base-64 the data in \p src and put the resulting string
> > + * in \p dst.
> > + *
> 
> > + * @param dst_len lenght of the \p dst string, it has to be at least
> > + * \p src_size * 4 / 3 + 12.
> 
> From where do you have this number?
> Is it due tp base64 or our implementation?

I read it in the implementation, and that limitation didn't made
sense to me.

Can you confirm that the attached patch is correct? (At least I can
confirm that it works with the test program.)

> API and implementation are 2 seperate things. The API should
> document exactly what is needed for both sides to interact, the implementation
> details should not be documented in any way in the public header.

OK, just let me note that in this case the implementation details
caused an unexpected failure.

Regards.
-- 
FFmpeg = Fostering and Fierce Mere Philosofic Encoding/decoding Ghost
-------------- next part --------------
A non-text attachment was scrubbed...
Name: base64-remove-encode-constraint.patch
Type: text/x-diff
Size: 569 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090125/ff2be9a3/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: doxy-for-base64-01.patch
Type: text/x-diff
Size: 1367 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090125/ff2be9a3/attachment-0001.patch>



More information about the ffmpeg-devel mailing list