diff mbox

[FFmpeg-devel,PATCHv2] vf_colorspace: Allow overriding input color properties

Message ID 20160829152346.80447-1-vittorio.giovara@gmail.com
State Superseded
Headers show

Commit Message

Vittorio Giovara Aug. 29, 2016, 3:23 p.m. UTC 
The filter needs input frames with color properties filled out by
the decoder. Since this is not always possible, add input options to
the filter so that user may override color space, color primaries,
transfer characteristics, and color range, as well as a generic option
to set all properties at once.

Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
---
* added iall option
* updated filter documentation

Please CC.
Vittorio

 doc/filters.texi            | 20 ++++++++++++++++++++
 libavfilter/vf_colorspace.c | 39 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 54 insertions(+), 5 deletions(-)

Comments

Ronald S. Bultje Aug. 29, 2016, 3:53 p.m. UTC | #1 
Hi,

On Mon, Aug 29, 2016 at 11:23 AM, Vittorio Giovara <
vittorio.giovara@gmail.com> wrote:

> The filter needs input frames with color properties filled out by
> the decoder. Since this is not always possible, add input options to
> the filter so that user may override color space, color primaries,
> transfer characteristics, and color range, as well as a generic option
> to set all properties at once.
>
> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
> ---
> * added iall option
> * updated filter documentation
>
> Please CC.
> Vittorio
>
>  doc/filters.texi            | 20 ++++++++++++++++++++
>  libavfilter/vf_colorspace.c | 39 ++++++++++++++++++++++++++++++++++-----
>  2 files changed, 54 insertions(+), 5 deletions(-)


lgtm.

(I wonder if the error message - if the input AVFrame has no
trc/rng/csp/prm - should be changed to reflect that you can override them
using the added properties? This isn't a big deal but maybe someone feels
that's important.)

Ronald
Paul B Mahol Aug. 30, 2016, 8:13 a.m. UTC | #2 
On Mon, Aug 29, 2016 at 5:53 PM, Ronald S. Bultje <rsbultje@gmail.com>
wrote:

> Hi,
>
> On Mon, Aug 29, 2016 at 11:23 AM, Vittorio Giovara <
> vittorio.giovara@gmail.com> wrote:
>
> > The filter needs input frames with color properties filled out by
> > the decoder. Since this is not always possible, add input options to
> > the filter so that user may override color space, color primaries,
> > transfer characteristics, and color range, as well as a generic option
> > to set all properties at once.
> >
> > Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
> > ---
> > * added iall option
> > * updated filter documentation
> >
> > Please CC.
> > Vittorio
> >
> >  doc/filters.texi            | 20 ++++++++++++++++++++
> >  libavfilter/vf_colorspace.c | 39 ++++++++++++++++++++++++++++++
> ++++-----
> >  2 files changed, 54 insertions(+), 5 deletions(-)
>
>
> lgtm.
>
> (I wonder if the error message - if the input AVFrame has no
> trc/rng/csp/prm - should be changed to reflect that you can override them
> using the added properties? This isn't a big deal but maybe someone feels
> that's important.)
>

Still breaks backward compatibility.
Ronald S. Bultje Aug. 30, 2016, 9:25 a.m. UTC | #3 
Hi,

On Tue, Aug 30, 2016 at 4:13 AM, Paul B Mahol <onemda@gmail.com> wrote:

> On Mon, Aug 29, 2016 at 5:53 PM, Ronald S. Bultje <rsbultje@gmail.com>
> wrote:
>
> > Hi,
> >
> > On Mon, Aug 29, 2016 at 11:23 AM, Vittorio Giovara <
> > vittorio.giovara@gmail.com> wrote:
> >
> > > The filter needs input frames with color properties filled out by
> > > the decoder. Since this is not always possible, add input options to
> > > the filter so that user may override color space, color primaries,
> > > transfer characteristics, and color range, as well as a generic option
> > > to set all properties at once.
> > >
> > > Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
> > > ---
> > > * added iall option
> > > * updated filter documentation
> > >
> > > Please CC.
> > > Vittorio
> > >
> > >  doc/filters.texi            | 20 ++++++++++++++++++++
> > >  libavfilter/vf_colorspace.c | 39 ++++++++++++++++++++++++++++++
> > ++++-----
> > >  2 files changed, 54 insertions(+), 5 deletions(-)
> >
> >
> > lgtm.
> >
> > (I wonder if the error message - if the input AVFrame has no
> > trc/rng/csp/prm - should be changed to reflect that you can override them
> > using the added properties? This isn't a big deal but maybe someone feels
> > that's important.)
> >
>
> Still breaks backward compatibility.


Hm yes that should probably be fixed. you said it just means moving the new
properties to the end of the AVOptions[] right?

Ronald
diff mbox

Patch

diff --git a/doc/filters.texi b/doc/filters.texi
index b50d7a6..8045840 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -5235,6 +5235,7 @@  Convert colorspace, transfer characteristics or color primaries.
 The filter accepts the following options:
 
 @table @option
+@anchor{all}
 @item all
 Specify all color properties at once.
 
@@ -5266,6 +5267,10 @@  BT.2020
 
 @end table
 
+@item iall
+Override all input properties at once. Same accepted values as @ref{all}.
+
+@anchor{space}
 @item space
 Specify output colorspace.
 
@@ -5291,6 +5296,10 @@  BT.2020 with non-constant luminance
 
 @end table
 
+@item ispace
+Override input colorspace. Same accepted values as @ref{space}.
+
+@anchor{trc}
 @item trc
 Specify output transfer characteristics.
 
@@ -5319,6 +5328,10 @@  BT.2020 for 12-bits content
 
 @end table
 
+@item itrc
+Override input transfer characteristics. Same accepted values as @ref{trc}.
+
+@anchor{primaries}
 @item primaries
 Specify output color primaries.
 
@@ -5344,6 +5357,10 @@  BT.2020
 
 @end table
 
+@item iprimaries
+Override input color primaries. Same accepted values as @ref{primaries}.
+
+@anchor{range}
 @item range
 Specify output color range.
 
@@ -5357,6 +5374,9 @@  JPEG (full) range
 
 @end table
 
+@item irange
+Override input color range. Same accepted values as @ref{range}.
+
 @item format
 Specify output color format.
 
diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
index e4022f8..c1ef5b3 100644
--- a/libavfilter/vf_colorspace.c
+++ b/libavfilter/vf_colorspace.c
@@ -128,11 +128,11 @@  typedef struct ColorSpaceContext {
 
     ColorSpaceDSPContext dsp;
 
-    enum Colorspace user_all;
-    enum AVColorSpace in_csp, out_csp, user_csp;
-    enum AVColorRange in_rng, out_rng, user_rng;
-    enum AVColorTransferCharacteristic in_trc, out_trc, user_trc;
-    enum AVColorPrimaries in_prm, out_prm, user_prm;
+    enum Colorspace user_all, user_iall;
+    enum AVColorSpace in_csp, out_csp, user_csp, user_icsp;
+    enum AVColorRange in_rng, out_rng, user_rng, user_irng;
+    enum AVColorTransferCharacteristic in_trc, out_trc, user_trc, user_itrc;
+    enum AVColorPrimaries in_prm, out_prm, user_prm, user_iprm;
     enum AVPixelFormat in_format, user_format;
     int fast_mode;
     enum DitherMode dither;
@@ -581,6 +581,10 @@  static int create_filtergraph(AVFilterContext *ctx,
 
     if (!s->out_primaries || !s->in_primaries) {
         s->in_prm = in->color_primaries;
+        if (s->user_iall != CS_UNSPECIFIED)
+            s->in_prm = default_prm[FFMIN(s->user_iall, CS_NB)];
+        if (s->user_iprm != AVCOL_PRI_UNSPECIFIED)
+            s->in_prm = s->user_iprm;
         s->in_primaries = get_color_primaries(s->in_prm);
         if (!s->in_primaries) {
             av_log(ctx, AV_LOG_ERROR,
@@ -638,6 +642,10 @@  static int create_filtergraph(AVFilterContext *ctx,
     if (!s->in_txchr) {
         av_freep(&s->lin_lut);
         s->in_trc = in->color_trc;
+        if (s->user_iall != CS_UNSPECIFIED)
+            s->in_trc = default_trc[FFMIN(s->user_iall, CS_NB)];
+        if (s->user_itrc != AVCOL_TRC_UNSPECIFIED)
+            s->in_trc = s->user_itrc;
         s->in_txchr = get_transfer_characteristics(s->in_trc);
         if (!s->in_txchr) {
             av_log(ctx, AV_LOG_ERROR,
@@ -680,7 +688,13 @@  static int create_filtergraph(AVFilterContext *ctx,
 
     if (!s->in_lumacoef) {
         s->in_csp = in->colorspace;
+        if (s->user_iall != CS_UNSPECIFIED)
+            s->in_csp = default_csp[FFMIN(s->user_iall, CS_NB)];
+        if (s->user_icsp != AVCOL_SPC_UNSPECIFIED)
+            s->in_csp = s->user_icsp;
         s->in_rng = in->color_range;
+        if (s->user_irng != AVCOL_RANGE_UNSPECIFIED)
+            s->in_rng = s->user_irng;
         s->in_lumacoef = get_luma_coefficients(s->in_csp);
         if (!s->in_lumacoef) {
             av_log(ctx, AV_LOG_ERROR,
@@ -1002,6 +1016,9 @@  static const AVOption colorspace_options[] = {
     { "all",        "Set all color properties together",
       OFFSET(user_all),   AV_OPT_TYPE_INT, { .i64 = CS_UNSPECIFIED },
       CS_UNSPECIFIED, CS_NB - 1, FLAGS, "all" },
+    { "iall",       "Set all input color properties together",
+      OFFSET(user_iall),   AV_OPT_TYPE_INT, { .i64 = CS_UNSPECIFIED },
+      CS_UNSPECIFIED, CS_NB - 1, FLAGS, "all" },
     ENUM("bt470m",      CS_BT470M,             "all"),
     ENUM("bt470bg",     CS_BT470BG,            "all"),
     ENUM("bt601-6-525", CS_BT601_6_525,        "all"),
@@ -1014,6 +1031,9 @@  static const AVOption colorspace_options[] = {
     { "space",      "Output colorspace",
       OFFSET(user_csp),   AV_OPT_TYPE_INT, { .i64 = AVCOL_SPC_UNSPECIFIED },
       AVCOL_PRI_RESERVED0, AVCOL_PRI_NB - 1, FLAGS, "csp" },
+    { "ispace",     "Input colorspace",
+      OFFSET(user_icsp),  AV_OPT_TYPE_INT, { .i64 = AVCOL_SPC_UNSPECIFIED },
+      AVCOL_PRI_RESERVED0, AVCOL_PRI_NB - 1, FLAGS, "csp" },
     ENUM("bt709",       AVCOL_SPC_BT709,       "csp"),
     ENUM("fcc",         AVCOL_SPC_FCC,         "csp"),
     ENUM("bt470bg",     AVCOL_SPC_BT470BG,     "csp"),
@@ -1024,12 +1044,18 @@  static const AVOption colorspace_options[] = {
     { "range",      "Output color range",
       OFFSET(user_rng),   AV_OPT_TYPE_INT, { .i64 = AVCOL_RANGE_UNSPECIFIED },
       AVCOL_RANGE_UNSPECIFIED, AVCOL_RANGE_NB - 1, FLAGS, "rng" },
+    { "irange",     "Input color range",
+      OFFSET(user_irng),  AV_OPT_TYPE_INT, { .i64 = AVCOL_RANGE_UNSPECIFIED },
+      AVCOL_RANGE_UNSPECIFIED, AVCOL_RANGE_NB - 1, FLAGS, "rng" },
     ENUM("mpeg",        AVCOL_RANGE_MPEG,      "rng"),
     ENUM("jpeg",        AVCOL_RANGE_JPEG,      "rng"),
 
     { "primaries",  "Output color primaries",
       OFFSET(user_prm),   AV_OPT_TYPE_INT, { .i64 = AVCOL_PRI_UNSPECIFIED },
       AVCOL_PRI_RESERVED0, AVCOL_PRI_NB - 1, FLAGS, "prm" },
+    { "iprimaries", "Input color primaries",
+      OFFSET(user_iprm),  AV_OPT_TYPE_INT, { .i64 = AVCOL_PRI_UNSPECIFIED },
+      AVCOL_PRI_RESERVED0, AVCOL_PRI_NB - 1, FLAGS, "prm" },
     ENUM("bt709",        AVCOL_PRI_BT709,      "prm"),
     ENUM("bt470m",       AVCOL_PRI_BT470M,     "prm"),
     ENUM("bt470bg",      AVCOL_PRI_BT470BG,    "prm"),
@@ -1040,6 +1066,9 @@  static const AVOption colorspace_options[] = {
     { "trc",        "Output transfer characteristics",
       OFFSET(user_trc),   AV_OPT_TYPE_INT, { .i64 = AVCOL_TRC_UNSPECIFIED },
       AVCOL_TRC_RESERVED0, AVCOL_TRC_NB - 1, FLAGS, "trc" },
+    { "itrc",       "Input transfer characteristics",
+      OFFSET(user_itrc),  AV_OPT_TYPE_INT, { .i64 = AVCOL_TRC_UNSPECIFIED },
+      AVCOL_TRC_RESERVED0, AVCOL_TRC_NB - 1, FLAGS, "trc" },
     ENUM("bt709",        AVCOL_TRC_BT709,        "trc"),
     ENUM("gamma22",      AVCOL_TRC_GAMMA22,      "trc"),
     ENUM("gamma28",      AVCOL_TRC_GAMMA28,      "trc"),