On Wed, 15 May 2013 23:15:24 +0200, Diego Biurrun <[email protected]> wrote:
> On 05/14/2013 09:37 AM, Anton Khirnov wrote:
> > --- a/libavfilter/avfilter.h
> > +++ b/libavfilter/avfilter.h
> > @@ -396,20 +396,41 @@ enum AVMediaType avfilter_pad_get_type(const
> > AVFilterPad *pads, int pad_idx);
> >
> > /**
> > - * A description for the filter. You should use the
> > - * NULL_IF_CONFIG_SMALL() macro to define it.
> > + * A description for the filter. May be NULL.
>
> I'd say s/of/for/.
>
> > - const AVFilterPad *inputs; ///< NULL terminated list of inputs. NULL
> > if none
> > - const AVFilterPad *outputs; ///< NULL terminated list of outputs. NULL
> > if none
> > + /**
> > + * List of inputs, terminated by a zeroed element.
> > + *
> > + * NULL if there are no (static) inputs. Instances of filters with
> > + * AVFILTER_FLAG_DYNAMIC_INPUTS set may have more inputs than present
> > in this list.
>
> Long it is, this line..
>
> > + /**
> > + * List of outputs, terminated by a zeroed element.
> > + *
> > + * NULL if there are no (static) outputs. Instances of filters with
> > + * AVFILTER_FLAG_DYNAMIC_OUTPUTS set may have more outputs than
> > present in
> > + * this list.
>
> .. unlike this one.
>
> > /**
> > - * A class for the private data, used to access filter private
> > - * AVOptions.
> > + * A class for the private data, used to declare filter private
> > AVOptions.
> > + * This field is NULL for the filters that do not declare any options.
>
> for filters
>
All the above fixed.
> > @@ -427,29 +448,71 @@ typedef struct AVFilter {
> >
> > /**
> > + * Filter initialization function.
> > + *
> > + * This callback will be called only once during the filter lifetime,
> > after
>
> Let's increase literary value via s/called/invoked/.
I think the documentation should be above all easy to understand. Invoke is a
much more obscure word than call, some non-native readers might have problems
with it. Also 'call a function' is a standard term, so i'd go with that.
>
> > /**
> > + * Should be set instead of @ref AVFilter.init "init" by the filters
> > that
> > + * want to pass a dictionary of AVOptions to nested contexts that are
> > + * allocated in init.
>
> All those inits have made my head spin; I cannot make heads or tails of
> this one.
>
> > /**
> > + * Query formats supported by the filter on its inputs and outputs.
> > + *
> > + * This callback is called after the filter is initialized (so the
> > inputs
> > + * and outputs are fixed), shortly before the format negotiation. This
> > + * callback may be called more than once.
>
> .. format negotiation and it may be invoked more than once.
Same here, I think shorter sentences are better. The preceding sentence is
already long enough.
>
> > + * This callback must set AVFilterLink.out_formats on every input link
> > and
> > + * AVFilterLink.in_formats on every output link to a list of supported
> > + * pixel/sample formats on that link.
>
> .. on every output link to a list of pixel/sample formats that the link
> supports.
It's not the link that supports the formats. A link has two ends, there is a
filter on each end. Each filter has its own list of supported formats.
>
> > + * This callback may be NULL for filters with one input, in which case
> > + * libavfilter assumes that it supports all formats on input and
> > preserves
>
> that it supports all input formats
Fixed.
>
> > @@ -458,6 +521,9 @@ typedef struct AVFilter {
> >
> > + /**
> > + * Used by the filter registration system.
> > + */
> > struct AVFilter *next;
>
> That's a tad terse.
That's intended. This field is only meant to be used by the registration
system, nobody else should touch it. So the doxy should just say that.
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
