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

[Michael Niedermayer]

On Fri, Jul 18, 2008 at 12:43:04AM +0200, Stefano Sabatini wrote:
> On date Thursday 2008-07-17 03:38:23 +0200, Michael Niedermayer encoded:
> > 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:
> > > > > 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
> 
> Fixed.
> 
> > > > > + *
> > > > > + * @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.
> 
> Fixed.
>  
> > > > > + * @param[in] name the name of the option contained in \p obj
> > > > > + * corresponding to the field to set.
> > > > 
> > > > the name of the field
> 
> Fixed.
> 
> > > 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)
> 
> Note that I don't agree with some of the objections, nonetheless I
> don't want to waste too much of our time in endless discussion
> achieving little or nothing, furthermore you're the boss afterall so I
> can't win ;-), also I'm quite satisfied with this patch, from 0 to
> some (partly inaccurate) documentation we're still getting a though
> improvement!

I leave further review of this one specific patch to diego. Iam not 100%
happy with it yet, a few parts sound still a little odd.

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

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- 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/20080718/14b3451f/attachment.pgp>