[FFmpeg-devel] [PATCH] libavfilter-soc: extend vf_scale.c to make it support colorspace details setting
Michael Niedermayer
michaelni
Fri May 15 21:35:13 CEST 2009
On Sat, May 02, 2009 at 02:23:42PM +0200, Stefano Sabatini wrote:
> On date Saturday 2009-04-18 20:00:41 +0200, Michael Niedermayer encoded:
> > On Sat, Apr 18, 2009 at 07:13:49PM +0200, Michael Niedermayer wrote:
> > > On Sat, Apr 18, 2009 at 06:03:13PM +0200, Vitor Sessak wrote:
> > > > Stefano Sabatini wrote:
> > > >> On date Thursday 2009-04-16 00:29:13 +0200, Stefano Sabatini encoded:
> > > >>> On date Wednesday 2009-04-15 22:56:27 +0200, Stefano Sabatini encoded:
> > > >>>> Hi, as in subject,
> > > >>>>
> > > >>>> maybe the patch should be split, also chroma horizontal shifting and
> > > >>>> chroma vertical shifting support should be added.
> > > >>>>
> > > >>>> Try it with:
> > > >>>> ffplay in.avi -vfilters
> > > >>>> "scale=0:0:sws_flags\=+print_info:lgb\=3.0:cgb\=3.0:brightness\=-20,
> > > >>>> format=rgb32"
> > > >>>>
> > > >>>> Note that the format=rgb32 or equivalent is required to make the
> > > >>>> destination format of the filter supported by
> > > >>>> sws_get/setColorspaceDetails(), or no filtering will be done (the
> > > >>>> filter still prints a warning in this case).
> > > >>>>
> > > >>>> BTW I get red chroma shifting when using format=argb.
> > > >>> Patch updated with some cleanups and chs/cvs support added.
> > > >
> > > > Sorry for the delay. Patch looks fine for me, but I think vf_scale is
> > > > getting more and more complex and could use some documentation in
> > > > vfilters.texi.
> > >
> > > also id like to say that people should try to move code from soc to
> > > main svn. The bigger the code in soc becomes the harder it will
> > > be to ever move this into main svn.
> > > Changes like this patch make the code less acceptable not more.
> > > Ive said it already, and i say it again, parameter parsing must be
> > > done cleanly, AVOptions are there and can be used, if you implement the
> > > same in 5 times as much code with strcmp & scanf you just added yourself
> > > reverting work if the code is supposed to reach main svn
> >
> > and to skip the "how do i" question
> > you have a struct you added variables there, now instead of a long list
> > of broken strstr()+scanf() that work by mere luck of not having some strings
> > occur you add a AVOptions table, split the string cleanly and use AVOptions
>
> Patchset for using av_set_options_string(), still missing
> documentation.
>
> I'm also planning to use some expression for scaled w and h, similar
> to what done for vf_pad.
>
> Regards.
> --
> FFmpeg = Faboulous and Funny Maxi Pacific Elected God
> vf_scale.c | 29 ++++++++++++++++++++++-------
> 1 file changed, 22 insertions(+), 7 deletions(-)
> a9bb7a0299379295d9cfbcf45297413d8616201d scale-add-parse-options-support.patch
> Index: libavfilter-soc/ffmpeg/libavfilter/vf_scale.c
> ===================================================================
> --- libavfilter-soc.orig/ffmpeg/libavfilter/vf_scale.c 2009-05-01 12:33:03.000000000 +0200
> +++ libavfilter-soc/ffmpeg/libavfilter/vf_scale.c 2009-05-01 19:48:47.000000000 +0200
> @@ -22,11 +22,12 @@
> #include <stdio.h>
>
> #include "avfilter.h"
> -#include "libavcodec/opt.h"
> +#include "parseutils.h"
> #include "libswscale/swscale.h"
>
> typedef struct
> {
> + const AVClass *av_class;
> struct SwsContext *sws; ///< software scaler context
>
> /**
> @@ -35,17 +36,29 @@
> * -1 = keep original aspect
> */
> int w, h;
> + char *sws_flags;
flags is no char*
>
> int sliceY; ///< top of current output slice
> } ScaleContext;
>
> +#define OFFSET(x) offsetof(ScaleContext, x)
> +
> +static const AVOption options[]={
> +{"sws_flags", "set SWScaler sws_flags", OFFSET(sws_flags), FF_OPT_TYPE_STRING, 0, CHAR_MIN, CHAR_MAX},
this is outright wrong, flags is no char* not in the actual implementation
nor semantically
> +{NULL},
> +};
> +
> +AV_DEFINE_CLASS(scale);
i have at least twice rejected this already
> +
> static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
> {
> ScaleContext *scale = ctx->priv;
> char sws_opts[256];
> - char *p;
>
> /* default to no scaling */
> + scale->av_class = &scale_class;
> + av_opt_set_defaults2(scale, 0, 0);
> +
> scale->w =
> scale->h = 0;
>
> @@ -55,11 +68,11 @@
> if(args)
> sscanf(args, "%d:%d:%255s", &scale->w, &scale->h, sws_opts);
>
> - if ((p = strstr(sws_opts, "sws_flags="))) {
> - char sws_flags[256];
> - sscanf(p, "sws_flags=%255[^:]", sws_flags);
> + if (av_set_options_string(scale, sws_opts, "=", ":") < 0)
> + return -1;
>
> - if (av_set_string3(scale->sws, "sws_flags", sws_flags, 1, NULL) < 0) {
> + if (scale->sws_flags) {
> + if (av_set_string3(scale->sws, "sws_flags", scale->sws_flags, 1, NULL) < 0) {
could we get rid of this sws_flags special case?
> sws_freeContext(scale->sws);
> scale->sws = NULL;
> return -1;
> @@ -79,8 +92,10 @@
> static av_cold void uninit(AVFilterContext *ctx)
> {
> ScaleContext *scale = ctx->priv;
> - if(scale->sws)
> + if(scale->sws) {
> + av_free(scale->sws_flags);
> sws_freeContext(scale->sws);
> + }
> }
>
> static int query_formats(AVFilterContext *ctx)
> vf_scale.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> ee0e0823bc8b5ea5b134e38809bba22f1fd7f414 scale-better-name-for-sws-opts.patch
> Index: libavfilter-soc/ffmpeg/libavfilter/vf_scale.c
> ===================================================================
> --- libavfilter-soc.orig/ffmpeg/libavfilter/vf_scale.c 2009-05-01 22:47:34.000000000 +0200
> +++ libavfilter-soc/ffmpeg/libavfilter/vf_scale.c 2009-05-02 13:40:56.000000000 +0200
> @@ -53,7 +53,7 @@
> static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
> {
> ScaleContext *scale = ctx->priv;
> - char sws_opts[256];
> + char extra_opts[256];
>
> /* default to no scaling */
> scale->av_class = &scale_class;
> @@ -66,9 +66,9 @@
> return -1;
>
> if(args)
> - sscanf(args, "%d:%d:%255s", &scale->w, &scale->h, sws_opts);
> + sscanf(args, "%d:%d:%255s", &scale->w, &scale->h, extra_opts);
>
> - if (av_set_options_string(scale, sws_opts, "=", ":") < 0)
> + if (av_set_options_string(scale, extra_opts, "=", ":") < 0)
> return -1;
>
> if (scale->sws_flags) {
> vf_scale.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
> 73e499a1b3e71f943252611d96841f033003d9f6 scale-support-cpuflags.patch
> Index: libavfilter-soc/ffmpeg/libavfilter/vf_scale.c
> ===================================================================
> --- libavfilter-soc.orig/ffmpeg/libavfilter/vf_scale.c 2009-05-02 13:40:56.000000000 +0200
> +++ libavfilter-soc/ffmpeg/libavfilter/vf_scale.c 2009-05-02 14:18:42.000000000 +0200
> @@ -23,6 +23,7 @@
>
> #include "avfilter.h"
> #include "parseutils.h"
> +#include "libavcodec/dsputil.h"
> #include "libswscale/swscale.h"
>
> typedef struct
> @@ -37,6 +38,7 @@
> */
> int w, h;
> char *sws_flags;
> + int auto_cpu_caps;
>
> int sliceY; ///< top of current output slice
> } ScaleContext;
> @@ -45,11 +47,22 @@
>
> static const AVOption options[]={
> {"sws_flags", "set SWScaler sws_flags", OFFSET(sws_flags), FF_OPT_TYPE_STRING, 0, CHAR_MIN, CHAR_MAX},
> +{"auto_cpu_caps", "enable SWScaler CPU caps automatic setting" , OFFSET(auto_cpu_caps), FF_OPT_TYPE_INT, 1, 0, 1},
> {NULL},
> };
>
> AV_DEFINE_CLASS(scale);
>
> +static av_cold int64_t get_sws_cpu_caps_flags(void) {
> + int64_t cpu_caps_flags = mm_support();
> +
> + return
> + (cpu_caps_flags & FF_MM_MMX ? SWS_CPU_CAPS_MMX : 0) |
> + (cpu_caps_flags & FF_MM_MMX2 ? SWS_CPU_CAPS_MMX2 : 0) |
> + (cpu_caps_flags & FF_MM_3DNOW ? SWS_CPU_CAPS_3DNOW : 0) |
> + (cpu_caps_flags & FF_MM_ALTIVEC ? SWS_CPU_CAPS_ALTIVEC : 0);
> +}
> +
> static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
> {
> ScaleContext *scale = ctx->priv;
> @@ -79,6 +92,11 @@
> }
> }
>
> + if (scale->auto_cpu_caps) {
> + int64_t sws_flags = av_get_int(scale->sws, "sws_flags", NULL);
> + av_set_int(scale->sws, "sws_flags", sws_flags | get_sws_cpu_caps_flags());
> + }
> +
> /* sanity check parms */
> if(scale->w < -1 || scale->h < -1)
> return -1;
we will have to find a cleaner solution for this and if none is found
that means this should stay out of svn until a clean solution is found.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
It is not what we do, but why we do it that matters.
-------------- 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/20090515/9b58251c/attachment.pgp>
More information about the ffmpeg-devel
mailing list