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

[Stefano Sabatini]

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!

Regards.
-- 
FFmpeg = Foolish & Faboulous Meaningful Portable Erotic Game
-------------- next part --------------
A non-text attachment was scrubbed...
Name: document-av-set-string-02.patch
Type: text/x-diff
Size: 1524 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080718/cc452469/attachment.patch>