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

Larry Robinson silver-dad
Mon Jan 10 06:20:21 CET 2011

On 1/9/2011 2:30 PM, Michael Niedermayer wrote:

On Sun, Jan 09, 2011 at 08:26:25PM +0100, Stefano Sabatini wrote:

On date Sunday 2011-01-09 19:42:27 +0100, Michael Niedermayer encoded:

On Sun, Jan 09, 2011 at 12:11:02AM +0100, Stefano Sabatini wrote:

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 configurefilters(...) in ffmpeg.c starts the filter chain on line 364 by calling avfiltergraphcreatefilter(&ist...). I propose to do the following: 1) Create a new data structure, located in cmdutils.h: typedef struct AVVolatileVideoProperties { enum AVColorPrimaries colorprimaries; enum AVColorTransferCharacteristic colortrc; enum AVColorSpace colorspace; enum AVColorRange colorrange; AVChromaLocation chromasamplelocation; } 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: setvolatilevideoproperties( 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 avfiltergraphcreatefilter, i.e. int avfiltergraphcreatefilter(AVFilterContext **filtctx, AVFilter *filt, const char *name, const char *args, void *opaque, AVFilterGraph *graphctx, 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. iam not so sure if opt.h is a good idea Maybe I could just add them to the arg string passed to the "buffer" filter init function. Then there's no change to AVFilterGraph, which addresses the next comment. Also I'm not sure colorspace should be a global property of the filter graph, rather than a property of the single link/filter. yes, agree here, and they could even change when the input changes (thats more a architectural design note and not a request to actually support it at first)

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 colorprimaries;

/** * Color Transfer Characteristic. * - encoding: Set by user * - decoding: Set by libavcodec */ enum AVColorTransferCharacteristic colortrc; /** * 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 colorrange; /** * This defines the location of chroma samples. * - encoding: Set by user * - decoding: Set by libavcodec */ enum AVChromaLocation chromasamplelocation; [...] 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 LIBAVFILTERVERSIONMAJOR 1 #define LIBAVFILTERVERSIONMINOR 72 @@ -614,6 +615,42 @@ * input link is assumed to be an unchangeable property. */ AVRational timebase; + + /** + * Chromaticity coordinates of the source primaries. + * - input link: Set by previous filter, read only + * - output link: May be changed by filter + */ + enum AVColorPrimaries colorprimaries; I want to avoid unconditional dependency of libavfilter on libavcodec. using enums from avcodec.h does not create a runtime dependancy so i dont see a problem I believe that assuming the existence of libavcodec headers when you compile a libavfilter application is wrong, indeed you could even you have a point of course, still 1. its largely stylistic nitpickery IMHO 2. we will have filters that will use libavcodec stuff like (i)dct / wavelets for postprocessing and noise reduction These filters will need libavcodec headers for compilation, at runtime dlopen can be used to avoid a dependancy. Thus IMHO we have thousands of things that are more important than moving these enums around but iam not against them being moved if someone wants to do it. Its a simple project to get my feet wet as I learn the architecture of ffmpeg so I've done it. The attached patch and new file implement a move of the colorspace enums into libavcore/colorspace.h I tested it on my system (x86_64-w64-mingw32) and it builds and passes all tests without error. compile/install libavfilter without installing libavcodec (configure --enable-libavfilter --disable-libavcodec), no problem here in a distro you could have libavfilter-dev and yet not libavcodec-dev. it wouldnt work, we dont have 4 independant configure scripts, and quadruplicating the build system across 5 split -dev packages seems like a bad idea and if you meant the distro simply physically removd libavcodec. We should nozt support this castration and if someone still wanted he can leave the header there as well if he already manually cuts things randomly away

ffmpeg-devel mailing list ffmpeg-devel at mplayerhq.hu https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel

-------------- next part -------------- An embedded and charset-unspecified text was scrubbed... Name: avcodec_with_colorspace_in_avcore.patch URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110109/1c19d3f4/attachment.asc> -------------- next part -------------- An embedded and charset-unspecified text was scrubbed... Name: colorspace.h URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110109/1c19d3f4/attachment.txt>

More information about the ffmpeg-devel mailing list