[FFmpeg-devel] [PATCH] Document libavcodec/opt.h

[Michael Niedermayer]

On Sat, Jun 07, 2008 at 10:49:02AM +0200, Stefano Sabatini wrote:
[...]
> Since I was at it I did also other modifications, namely:
> * reworded the description of the return for av_next_option()
> * reworded the main description of av_get_*, now it sounds like:
> 
> Looks for and option in \p ctx and gets its value
> 
> rather than:
> 
> Gets an option value looked up in \p ctx
> (the use of the verb "to look up" was wrong).
> 
> Also I changed all the return definitions for av_get_*, which were
> *wrong* before as I wrote them (they don't return negative values, but
> null values in case of error or if no option has been found), I'm very
> sorry for this continuos modifications ballet, I think now it should
> be quite enough correct and accurate.
> 
> Best regards.
> -- 
> FFmpeg = Free & Fucked MultiPurpose EntanGlement

> Index: libavcodec/opt.h
> ===================================================================
> --- libavcodec/opt.h	(revision 13684)
> +++ libavcodec/opt.h	(working copy)
[...]
> @@ -45,6 +57,12 @@
>   * AVOption
>   */
>  typedef struct AVOption {
> +    /**
> +     * the name of the option<br>

The name of the option. (The dot is significant syntax wise)


> +     * It should be unique amongst the options contained in an AVClass
> +     * object. 


> +     * If the option contains a named value, then this field
> +     * contains the name of it.

redundant

> +     */
>      const char *name;
>  
>      /**
> @@ -52,12 +70,24 @@
>       * @todo What about other languages?
>       */
>      const char *help;
> -    int offset;             ///< offset to context structure where the parsed value should be stored
> +
> +    /**
> +     * the offset relative to the context structure where the option
> +     * value should be stored<br>

it should end in a dot not <br> !
<br> says line break, . says end of short explanation at least it was that
way last time i RTFM, i dunno if <br> works as well nowadays



> +     * It should be 0 if the option is used to contain a named value.

named constant


> +     */
> +    int offset;
>      enum AVOptionType type;
>  
> +    /**
> +     * the default value<br>
> +     * It is settable only for non string values. If the option is a
> +     * constant (for example is a named value) then it contains the
> +     * value of the constant.
> +     */
>      double default_val;

> -    double min;
> -    double max;
> +    double min;                 ///< minimum acceptable value for the option if the option does not contain a constant
> +    double max;                 ///< maximum acceptable value for the option if the option does not contain a constant

valid not acceptable
and id drop the "if the option does not contain a constant"


>  
>      int flags;
>  #define AV_OPT_FLAG_ENCODING_PARAM  1   ///< a generic parameter which can be set by the user for muxing or encoding
> @@ -67,22 +97,174 @@
>  #define AV_OPT_FLAG_VIDEO_PARAM     16
>  #define AV_OPT_FLAG_SUBTITLE_PARAM  32
>  //FIXME think about enc-audio, ... style flags

> +
> +    /**
> +     * Aggregates options into a single logical unit (for example
> +     * named values for a flag or an int). If the option contains a
> +     * named value then the named value is relative to the option with
> +     * the name equal to \p unit.
> +     */
>      const char *unit;

ugh, just remove that doxy please! You can try again in a seperate patch
maybe ...


>  } AVOption;
>  
> +/**
> + * Looks for an option in \p obj. Looks only for the options which

> + * have the flags masked with \p mask equal to \p flags (that is for
> + * which is true: opt->flags & mask = flags).

urhm ....
have the flags set as specified in mask and flags.


> + *
> + * @param[in] obj a pointer to an AVClass object
> + * @param[in] name the name of the option to look for

> + * @param[in] unit the unit of the option, if non-NULL only looks for
> + * options belonging to this unit

the unit of the option to look for or any if NULL
where is the wanderer when one needs him :(
Your documentations are sooooooooooooooooooo confused
they really could benefit from some heavy reformulations and i hate
reformulating docs


> + * @return a pointer to the first option found or NULL if no option
> + * has been found

s/first //

please split the patch! Lets try to get the above svn ready before the
rest below :)

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- 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/20080608/42cf8518/attachment.pgp>