[FFmpeg-devel] [PATCH] avfilter/showcqt: add csp option

Michael Niedermayer michael at niedermayer.cc
Fri Oct 14 04:47:47 EEST 2016


On Fri, Oct 14, 2016 at 08:05:15AM +0700, Muhammad Faiz wrote:
> On Fri, Oct 14, 2016 at 3:20 AM, Michael Niedermayer
> <michael at niedermayer.cc> wrote:
> > On Fri, Oct 14, 2016 at 02:50:52AM +0700, Muhammad Faiz wrote:
> >> On Thu, Oct 13, 2016 at 10:38 PM, Michael Niedermayer
> >> <michael at niedermayer.cc> wrote:
> >> > On Thu, Oct 13, 2016 at 06:05:19AM +0700, Muhammad Faiz wrote:
> >> >> from colorspace filter
> >> >>
> >> >> Signed-off-by: Muhammad Faiz <mfcc64 at gmail.com>
> >> >> ---
> >> >>  doc/filters.texi          | 26 ++++++++++++++++++++++
> >> >>  libavfilter/avf_showcqt.c | 56 ++++++++++++++++++++++++++++++++++++++++++-----
> >> >>  libavfilter/avf_showcqt.h |  2 ++
> >> >>  3 files changed, 79 insertions(+), 5 deletions(-)
> >> >>
> >> >> diff --git a/doc/filters.texi b/doc/filters.texi
> >> >> index 76265e7..a79972b 100644
> >> >> --- a/doc/filters.texi
> >> >> +++ b/doc/filters.texi
> >> >> @@ -16884,6 +16884,32 @@ Enable/disable drawing text to the axis. If it is set to @code{0}, drawing to
> >> >>  the axis is disabled, ignoring @var{fontfile} and @var{axisfile} option.
> >> >>  Default value is @code{1}.
> >> >>
> >> >> + at item csp
> >> >> +Set colorspace. The accepted values are:
> >> >> + at table @samp
> >> >> + at item unspecified
> >> >> +Unspecified (default)
> >> >> +
> >> >> + at item bt709
> >> >> +BT.709
> >> >> +
> >> >> + at item fcc
> >> >> +FCC
> >> >> +
> >> >> + at item bt470bg
> >> >> +BT.470BG or BT.601-6 625
> >> >> +
> >> >> + at item smpte170m
> >> >> +SMPTE-170M or BT.601-6 525
> >> >> +
> >> >> + at item smpte240m
> >> >> +SMPTE-240M
> >> >> +
> >> >> + at item bt2020ncl
> >> >> +BT.2020 with non-constant luminance
> >> >> +
> >> >> + at end table
> >> >> +
> >> >>  @end table
> >> >>
> >> >>  @subsection Examples
> >> >> diff --git a/libavfilter/avf_showcqt.c b/libavfilter/avf_showcqt.c
> >> >> index 16bb2be..7c76b1f 100644
> >> >> --- a/libavfilter/avf_showcqt.c
> >> >> +++ b/libavfilter/avf_showcqt.c
> >> >> @@ -83,6 +83,14 @@ static const AVOption showcqt_options[] = {
> >> >>      { "axisfile",     "set axis image", OFFSET(axisfile),  AV_OPT_TYPE_STRING, { .str = NULL },      CHAR_MIN, CHAR_MAX, FLAGS },
> >> >>      { "axis",              "draw axis", OFFSET(axis),        AV_OPT_TYPE_BOOL, { .i64 = 1 },                0, 1,        FLAGS },
> >> >>      { "text",              "draw axis", OFFSET(axis),        AV_OPT_TYPE_BOOL, { .i64 = 1 },                0, 1,        FLAGS },
> >> >> +    { "csp",         "set color space", OFFSET(csp),          AV_OPT_TYPE_INT, { .i64 = AVCOL_SPC_UNSPECIFIED }, 0, INT_MAX, FLAGS, "csp" },
> >> >> +        { "unspecified", "unspecified", 0,                  AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_UNSPECIFIED }, 0, 0, FLAGS, "csp" },
> >> >> +        { "bt709",             "bt709", 0,                  AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT709 },       0, 0, FLAGS, "csp" },
> >> >> +        { "fcc",                 "fcc", 0,                  AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_FCC },         0, 0, FLAGS, "csp" },
> >> >> +        { "bt470bg",         "bt470bg", 0,                  AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT470BG },     0, 0, FLAGS, "csp" },
> >> >> +        { "smpte170m",     "smpte170m", 0,                  AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_SMPTE170M },   0, 0, FLAGS, "csp" },
> >> >> +        { "smpte240m",     "smpte240m", 0,                  AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_SMPTE240M },   0, 0, FLAGS, "csp" },
> >> >> +        { "bt2020ncl",     "bt2020ncl", 0,                  AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT2020_NCL },  0, 0, FLAGS, "csp" },
> >> >>      { NULL }
> >> >>  };
> >> >>
> >> >> @@ -656,7 +664,7 @@ static void rgb_from_cqt(ColorFloat *c, const FFTComplex *v, float g, int len)
> >> >>      }
> >> >>  }
> >> >>
> >> >> -static void yuv_from_cqt(ColorFloat *c, const FFTComplex *v, float gamma, int len)
> >> >> +static void yuv_from_cqt(ColorFloat *c, const FFTComplex *v, float gamma, int len, float cm[3][3])
> >> >>  {
> >> >>      int x;
> >> >>      for (x = 0; x < len; x++) {
> >> >> @@ -664,9 +672,9 @@ static void yuv_from_cqt(ColorFloat *c, const FFTComplex *v, float gamma, int le
> >> >>          r = calculate_gamma(FFMIN(1.0f, v[x].re), gamma);
> >> >>          g = calculate_gamma(FFMIN(1.0f, 0.5f * (v[x].re + v[x].im)), gamma);
> >> >>          b = calculate_gamma(FFMIN(1.0f, v[x].im), gamma);
> >> >> -        c[x].yuv.y = 65.481f * r + 128.553f * g + 24.966f * b;
> >> >> -        c[x].yuv.u = -37.797f * r - 74.203f * g + 112.0f * b;
> >> >> -        c[x].yuv.v = 112.0f * r - 93.786f * g - 18.214 * b;
> >> >> +        c[x].yuv.y = cm[0][0] * r + cm[0][1] * g + cm[0][2] * b;
> >> >> +        c[x].yuv.u = cm[1][0] * r + cm[1][1] * g + cm[1][2] * b;
> >> >> +        c[x].yuv.v = cm[2][0] * r + cm[2][1] * g + cm[2][2] * b;
> >> >>      }
> >> >>  }
> >> >>
> >> >> @@ -1036,7 +1044,7 @@ static void process_cqt(ShowCQTContext *s)
> >> >>      if (s->format == AV_PIX_FMT_RGB24)
> >> >>          rgb_from_cqt(s->c_buf, s->cqt_result, s->sono_g, s->width);
> >> >>      else
> >> >> -        yuv_from_cqt(s->c_buf, s->cqt_result, s->sono_g, s->width);
> >> >> +        yuv_from_cqt(s->c_buf, s->cqt_result, s->sono_g, s->width, s->cmatrix);
> >> >>  }
> >> >>
> >> >>  static int plot_cqt(AVFilterContext *ctx, AVFrame **frameout)
> >> >> @@ -1075,6 +1083,7 @@ static int plot_cqt(AVFilterContext *ctx, AVFrame **frameout)
> >> >>              return AVERROR(ENOMEM);
> >> >>          out->sample_aspect_ratio = av_make_q(1, 1);
> >> >>          av_frame_set_color_range(out, AVCOL_RANGE_MPEG);
> >> >> +        av_frame_set_colorspace(out, s->csp);
> >> >>          UPDATE_TIME(s->alloc_time);
> >> >>
> >> >>          if (s->bar_h) {
> >> >
> >> >     > @@ -1100,6 +1109,41 @@ static int plot_cqt(AVFilterContext *ctx, AVFrame **frameout)
> >> >>      return 0;
> >> >>  }
> >> >>
> >> >> +static void init_colormatrix(ShowCQTContext *s)
> >> >> +{
> >> >> +    double kr, kg, kb;
> >> >> +
> >> >> +    /* from vf_colorspace.c */
> >> >> +    switch (s->csp) {
> >> >> +    default:
> >> >> +        av_log(s->ctx, AV_LOG_WARNING, "unsupported colorspace, setting it to unspecified.\n");
> >> >> +        s->csp = AVCOL_SPC_UNSPECIFIED;
> >> >> +    case AVCOL_SPC_UNSPECIFIED:
> >> >> +    case AVCOL_SPC_BT470BG:
> >> >> +    case AVCOL_SPC_SMPTE170M:
> >> >> +        kr = 0.299; kb = 0.114; break;
> >> >> +    case AVCOL_SPC_BT709:
> >> >> +        kr = 0.2126; kb = 0.0722; break;
> >> >> +    case AVCOL_SPC_FCC:
> >> >> +        kr = 0.30; kb = 0.11; break;
> >> >> +    case AVCOL_SPC_SMPTE240M:
> >> >> +        kr = 0.212; kb = 0.087; break;
> >> >> +    case AVCOL_SPC_BT2020_NCL:
> >> >> +        kr = 0.2627; kb = 0.0593; break;
> >> >> +    }
> >> >> +
> >> >> +    kg = 1.0 - kr - kb;
> >> >> +    s->cmatrix[0][0] = 219.0 * kr;
> >> >> +    s->cmatrix[0][1] = 219.0 * kg;
> >> >> +    s->cmatrix[0][2] = 219.0 * kb;
> >> >> +    s->cmatrix[1][0] = -112.0 * kr / (1.0 - kb);
> >> >> +    s->cmatrix[1][1] = -112.0 * kg / (1.0 - kb);
> >> >> +    s->cmatrix[1][2] = 112.0;
> >> >> +    s->cmatrix[2][0] = 112.0;
> >> >> +    s->cmatrix[2][1] = -112.0 * kg / (1.0 - kr);
> >> >> +    s->cmatrix[2][2] = -112.0 * kb / (1.0 - kr);
> >> >> +}
> >> >
> >> > hardcoding these numbers here feels a bit "ugly"
> >> > (or rather hardcoding them everywhere where they are used)
> >> >
> >> > these should be a #define or something like that from some header
> >> > and that named identifer(s) then used everywhere
> >> >
> >>
> >> 219 and 224 are used previously in many places and they are unnamed.
> >
> > yes, i was thinking that you could add appropriate defines to
> > libavutil/colorspace.h
> > if theres nothing suitable in it already
> > maybe more than just the #define would make sense to be moved over
> > to be shareable ...
> 
> macro/function to convert kr, kg, kb, maybe?
> or hardcoded rgb2yuv/yuv2rgb_table for each colorspace?
> 
> of course this is out of scope of this patch
> it should be on separate patch
> probably, I would not post that patch
> because it requires long discussion and I am not
> so active on mailing list

well, iam not objecting to your original patch if you want to leave
this for someone else / later.

just seemed to me that this stuff being used at multiple places
could be factored, that can be done later instead of course ...
and especially if it would end up blocking your work then probably
is better to do later ...


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

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161014/4e26fbe8/attachment.sig>


More information about the ffmpeg-devel mailing list