[FFmpeg-devel] [PATCH] Document opt.h:av_set_string functions

[Michael Niedermayer]

On Thu, Jul 17, 2008 at 12:59:50AM +0200, Stefano Sabatini wrote:
> On date Wednesday 2008-07-16 18:42:18 +0200, Michael Niedermayer encoded:
> > On Tue, Jul 15, 2008 at 11:10:14PM +0200, Stefano Sabatini wrote:
> > > On date Wednesday 2008-07-09 01:11:27 +0200, Michael Niedermayer encoded:
> > > > On Tue, Jul 08, 2008 at 09:47:47PM +0200, Stefano Sabatini wrote:
> > > > > On date Sunday 2008-07-06 11:00:34 +0200, Stefano Sabatini encoded:
> > > > > > Hi all, as in subject.
> > > > > > -- 
> > > > > > FFmpeg = Funny and Fantastic Multimedia Portable Educated Game
> > > > > 
> > > > [...]
> > > > > Please can you comment on this? Or I'll commit it on Saturday.
> > > > 
> > > > Would you accept a comment like
> > > > all your doxy improvments are rejected?
> > > > 
> > > > It costs me more time fixing them up than writing them from scratch
> > > 
> > > Michael, I'll try to avoid to write doxy at least not related to my
> > > own code (yes it's getting more and more a boring time waste), only
> > > please let me end up just the doxyfication of this file which is in my
> > > todo list since too much time and I don't want to waste all the effort
> > > and comments spent since then.
> > > 
> > > New try attached.
> > > 
> > > Thanks for your patience, regards.
> > > -- 
> > > FFmpeg = Funny & Frenzy Multimedia Powered Experimenting Guru
> > 
> > > Index: libavcodec/opt.h
> > > ===================================================================
> > > --- libavcodec/opt.h	(revision 14247)
> > > +++ libavcodec/opt.h	(working copy)
> > > @@ -99,10 +99,28 @@
> > >   */
> > >  const AVOption *av_find_opt(void *obj, const char *name, const char *unit, int mask, int flags);
> > >  
> > > +/**
> > > + * @see av_set_string2()
> > > + */
> > >  attribute_deprecated const AVOption *av_set_string(void *obj, const char *name, const char *val);
> > 
> > ok
> > 
> > >  
> > >  /**
> > > - * Sets the field of obj with the given name to value.
> > > + * Sets a field of \p obj with the value in \p val.
> > 
> > not ok
> 
> I think as I wrote it is more correct/clear. See below.

I dont think so


> 
> > > + *
> > > + * @param[in] obj a pointer to a struct whose first element is a
> > > + * pointer to an #AVClass
> > 
> > a struct whose first element is a pointer to an #AVClass
> 
> But is it a pointer no? Also this is inconsistent with what we already
> documented in av_find_opt().

The documentation is supposed to be readable, when in doubt theres always
the code.


> 
> > > + * @param[in] name the name of the option contained in \p obj
> > > + * corresponding to the field to set.
> > 
> > the name of the field
> 
> I think it's more correct as I wrote, name corresponds to the name of
> the AVOption, which can and often *is* different from the name of the
> field to set:

Well, it should not be different ideally ...

besides you insisted above that obj is not a struct but a pointer to a
struct.
What are options contained in a pointer to a struct?
(not that i know what options contained in a struct are)


[...]

> Thanks again for your review, just another little effort and we'll get
> to the end, 

> then I'll need a vacation period ;-).

me too ;)


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080717/40fe5f63/attachment.pgp>