[FFmpeg-devel] [PATCH] Re: How can I determine color_range from a filter?

[Stefano Sabatini]

On date Friday 2011-01-07 11:12:59 -0800, Larry Robinson encoded:
> So here's the easy part, adding the new properties to AVFilterLink.
> Let me know if this is inconsistent with style, architecture, etc.
> 
> Now - how to fill them:
> It looks like configure_filters(...) in ffmpeg.c starts the filter
> chain on line 364 by calling avfilter_graph_create_filter(&ist...).
> I propose to do the following:
> 
> 1) Create a new data structure, located in cmdutils.h:
>     typedef struct AVVolatileVideoProperties
>     {
>         enum AVColorPrimaries color_primaries;
>         enum AVColorTransferCharacteristic color_trc;
>         enum AVColorSpace colorspace;
>         enum AVColorRange color_range;
>         AVChromaLocation chroma_sample_location;
>     } AVVolatileVideoProperties;
> 
> NOTE:
> If I do this, cmdutils.h will have to be included after avcodec.h,
> because that's where AVColorPrimaries, etc. are declared.
> The second option is to  #include avcodec.h  in cmdutils.h - this
> will prevent breaking code that includes cmdutils but not avcodec.
> The last option is to declare the structure in both ffmpeg.c and
> ffplay.c and keep it out of cmdutils.h altogether.
> 
> Is there any guidance to help me make the decision?

I can't see the problem, cmdutils.h already includes avcodec.h.
 
> 2) Add a new function:  set_volatile_video_properties(
> AVCodecContext * ctx )  to {cmdutils.h, cmdutils.c}
>     which will copy the values into the AVVolatileVideoProperties
> structure.
> 
> The issues and options noted in (1) apply here also.  Guidance would
> be appreciated.
> 
> 3) Add an additional parameter to avfilter_graph_create_filter, i.e.
> 
> int avfilter_graph_create_filter(AVFilterContext **filt_ctx, AVFilter *filt,
>                                  const char *name, const char *args,
> void *opaque,
>                                  AVFilterGraph *graph_ctx,
> AVVolatileVideoProperties *vvp);
> 
> If vvp is not NULL, the properties in vvp will be copied to the
> AVFilterLink.
> 
> 4) The new properties will need to be propagated along the filter
> chain (I'll figure out how to do this), and ultimately used by
> encoders (beyond my expertise).

Maybe a better solution would be to use the option (libavutil/opt.h)
system in AVFilterGraph.

Also I'm not sure colorspace should be a global property of the filter
graph, rather than a property of the single link/filter.

> On 1/6/2011 6:28 PM, Michael Niedermayer wrote:
> >On Thu, Jan 06, 2011 at 04:06:33PM -0800, Larry Robinson wrote:
> >>Let me know what the (few others) are and I will try to add them :-) .
> >     /**
> >      * Chromaticity coordinates of the source primaries.
> >      * - encoding: Set by user
> >      * - decoding: Set by libavcodec
> >      */
> >     enum AVColorPrimaries color_primaries;
> >
> >     /**
> >      * Color Transfer Characteristic.
> >      * - encoding: Set by user
> >      * - decoding: Set by libavcodec
> >      */
> >     enum AVColorTransferCharacteristic color_trc;
> >
> >     /**
> >      * YUV colorspace type.
> >      * - encoding: Set by user
> >      * - decoding: Set by libavcodec
> >      */
> >     enum AVColorSpace colorspace;
> >
> >     /**
> >      * MPEG vs JPEG YUV range.
> >      * - encoding: Set by user
> >      * - decoding: Set by libavcodec
> >      */
> >     enum AVColorRange color_range;
> >
> >     /**
> >      * This defines the location of chroma samples.
> >      * - encoding: Set by user
> >      * - decoding: Set by libavcodec
> >      */
> >     enum AVChromaLocation chroma_sample_location;
[...]

> Index: libavfilter/avfilter.h
> ===================================================================
> --- libavfilter/avfilter.h	(revision 26253)
> +++ libavfilter/avfilter.h	(working copy)
> @@ -25,6 +25,7 @@
>  #include "libavutil/avutil.h"
>  #include "libavcore/avcore.h"
>  #include "libavcore/samplefmt.h"
> +#include "libavcodec/avcodec.h"
>  
>  #define LIBAVFILTER_VERSION_MAJOR  1
>  #define LIBAVFILTER_VERSION_MINOR 72
> @@ -614,6 +615,42 @@
>       * input link is assumed to be an unchangeable property.
>       */
>      AVRational time_base;
> +
> +    /**
> +     * Chromaticity coordinates of the source primaries.
> +     * - input link: Set by previous filter, read only
> +     * - output link: May be changed by filter
> +     */
> +    enum AVColorPrimaries color_primaries;

I want to avoid unconditional dependency of libavfilter on libavcodec.
My proposal is to move AVColor* definitions to libavcore, e.g. in
avcore.h (or maybe in a new header libavcore/color.h), where they
could be shared with libswscale as well.
-- 
FFmpeg = Fancy Fundamental Murdering Powerful Exciting Gadget