diff mbox

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

Message ID 1476313519-4109-1-git-send-email-mfcc64@gmail.com
State Accepted
Commit a11757d7cba97ea8625bfda0a10db2a2b8e66b02
Headers show

Commit Message

Muhammad Faiz Oct. 12, 2016, 11:05 p.m. UTC
from colorspace filter

Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
---
 doc/filters.texi          | 26 ++++++++++++++++++++++
 libavfilter/avf_showcqt.c | 56 ++++++++++++++++++++++++++++++++++++++++++-----
 libavfilter/avf_showcqt.h |  2 ++
 3 files changed, 79 insertions(+), 5 deletions(-)

Comments

Michael Niedermayer Oct. 13, 2016, 3:38 p.m. UTC | #1
On Thu, Oct 13, 2016 at 06:05:19AM +0700, Muhammad Faiz wrote:
> from colorspace filter
> 
> Signed-off-by: Muhammad Faiz <mfcc64@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}.
>  
> +@item csp
> +Set colorspace. The accepted values are:
> +@table @samp
> +@item unspecified
> +Unspecified (default)
> +
> +@item bt709
> +BT.709
> +
> +@item fcc
> +FCC
> +
> +@item bt470bg
> +BT.470BG or BT.601-6 625
> +
> +@item smpte170m
> +SMPTE-170M or BT.601-6 525
> +
> +@item smpte240m
> +SMPTE-240M
> +
> +@item bt2020ncl
> +BT.2020 with non-constant luminance
> +
> +@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


[...]
Muhammad Faiz Oct. 13, 2016, 7:50 p.m. UTC | #2
On Thu, Oct 13, 2016 at 10:38 PM, Michael Niedermayer
<michael@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@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}.
>>
>> +@item csp
>> +Set colorspace. The accepted values are:
>> +@table @samp
>> +@item unspecified
>> +Unspecified (default)
>> +
>> +@item bt709
>> +BT.709
>> +
>> +@item fcc
>> +FCC
>> +
>> +@item bt470bg
>> +BT.470BG or BT.601-6 625
>> +
>> +@item smpte170m
>> +SMPTE-170M or BT.601-6 525
>> +
>> +@item smpte240m
>> +SMPTE-240M
>> +
>> +@item bt2020ncl
>> +BT.2020 with non-constant luminance
>> +
>> +@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.
Michael Niedermayer Oct. 13, 2016, 8:20 p.m. UTC | #3
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@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@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}.
> >>
> >> +@item csp
> >> +Set colorspace. The accepted values are:
> >> +@table @samp
> >> +@item unspecified
> >> +Unspecified (default)
> >> +
> >> +@item bt709
> >> +BT.709
> >> +
> >> +@item fcc
> >> +FCC
> >> +
> >> +@item bt470bg
> >> +BT.470BG or BT.601-6 625
> >> +
> >> +@item smpte170m
> >> +SMPTE-170M or BT.601-6 525
> >> +
> >> +@item smpte240m
> >> +SMPTE-240M
> >> +
> >> +@item bt2020ncl
> >> +BT.2020 with non-constant luminance
> >> +
> >> +@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 ...

[...]
Muhammad Faiz Oct. 14, 2016, 1:05 a.m. UTC | #4
On Fri, Oct 14, 2016 at 3:20 AM, Michael Niedermayer
<michael@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@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@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}.
>> >>
>> >> +@item csp
>> >> +Set colorspace. The accepted values are:
>> >> +@table @samp
>> >> +@item unspecified
>> >> +Unspecified (default)
>> >> +
>> >> +@item bt709
>> >> +BT.709
>> >> +
>> >> +@item fcc
>> >> +FCC
>> >> +
>> >> +@item bt470bg
>> >> +BT.470BG or BT.601-6 625
>> >> +
>> >> +@item smpte170m
>> >> +SMPTE-170M or BT.601-6 525
>> >> +
>> >> +@item smpte240m
>> >> +SMPTE-240M
>> >> +
>> >> +@item bt2020ncl
>> >> +BT.2020 with non-constant luminance
>> >> +
>> >> +@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

thanks
Michael Niedermayer Oct. 14, 2016, 1:47 a.m. UTC | #5
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@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@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@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}.
> >> >>
> >> >> +@item csp
> >> >> +Set colorspace. The accepted values are:
> >> >> +@table @samp
> >> >> +@item unspecified
> >> >> +Unspecified (default)
> >> >> +
> >> >> +@item bt709
> >> >> +BT.709
> >> >> +
> >> >> +@item fcc
> >> >> +FCC
> >> >> +
> >> >> +@item bt470bg
> >> >> +BT.470BG or BT.601-6 625
> >> >> +
> >> >> +@item smpte170m
> >> >> +SMPTE-170M or BT.601-6 525
> >> >> +
> >> >> +@item smpte240m
> >> >> +SMPTE-240M
> >> >> +
> >> >> +@item bt2020ncl
> >> >> +BT.2020 with non-constant luminance
> >> >> +
> >> >> +@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 ...


[...]
Muhammad Faiz Oct. 15, 2016, 10:44 p.m. UTC | #6
On Fri, Oct 14, 2016 at 8:47 AM, Michael Niedermayer
<michael@niedermayer.cc> wrote:
> 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@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@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@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}.
>> >> >>
>> >> >> +@item csp
>> >> >> +Set colorspace. The accepted values are:
>> >> >> +@table @samp
>> >> >> +@item unspecified
>> >> >> +Unspecified (default)
>> >> >> +
>> >> >> +@item bt709
>> >> >> +BT.709
>> >> >> +
>> >> >> +@item fcc
>> >> >> +FCC
>> >> >> +
>> >> >> +@item bt470bg
>> >> >> +BT.470BG or BT.601-6 625
>> >> >> +
>> >> >> +@item smpte170m
>> >> >> +SMPTE-170M or BT.601-6 525
>> >> >> +
>> >> >> +@item smpte240m
>> >> >> +SMPTE-240M
>> >> >> +
>> >> >> +@item bt2020ncl
>> >> >> +BT.2020 with non-constant luminance
>> >> >> +
>> >> >> +@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 ...
>

pushed

thanks
diff mbox

Patch

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}.
 
+@item csp
+Set colorspace. The accepted values are:
+@table @samp
+@item unspecified
+Unspecified (default)
+
+@item bt709
+BT.709
+
+@item fcc
+FCC
+
+@item bt470bg
+BT.470BG or BT.601-6 625
+
+@item smpte170m
+SMPTE-170M or BT.601-6 525
+
+@item smpte240m
+SMPTE-240M
+
+@item bt2020ncl
+BT.2020 with non-constant luminance
+
+@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);
+}
+
 /* main filter control */
 static av_cold int init(AVFilterContext *ctx)
 {
@@ -1153,6 +1197,8 @@  static av_cold int init(AVFilterContext *ctx)
         } while(s->fcount * s->width < 1920 && s->fcount < 10);
     }
 
+    init_colormatrix(s);
+
     return 0;
 }
 
diff --git a/libavfilter/avf_showcqt.h b/libavfilter/avf_showcqt.h
index 588830f..71e9d13 100644
--- a/libavfilter/avf_showcqt.h
+++ b/libavfilter/avf_showcqt.h
@@ -71,6 +71,7 @@  typedef struct {
     float               *rcp_h_buf;
     float               *sono_v_buf;
     float               *bar_v_buf;
+    float               cmatrix[3][3];
     /* callback */
     void                (*cqt_calc)(FFTComplex *dst, const FFTComplex *src, const Coeffs *coeffs,
                                     int len, int fft_len);
@@ -111,6 +112,7 @@  typedef struct {
     char                *fontcolor;
     char                *axisfile;
     int                 axis;
+    int                 csp;
 } ShowCQTContext;
 
 void ff_showcqt_init_x86(ShowCQTContext *s);