[FFmpeg-devel] colorspace: Rename jedec-p22 to ebu3213

From: Raphaël Zumer <rzumer@tebako.net>

Internally, this adds an EBU3213 alias to JEDEC_P22,
and changes the name string to match ITU-T H.273.
---
 libavcodec/options_table.h  | 2 +-
 libavfilter/vf_colorspace.c | 2 +-
 libavfilter/vf_setparams.c  | 2 +-
 libavfilter/vf_zscale.c     | 2 +-
 libavutil/pixdesc.c         | 2 +-
 libavutil/pixfmt.h          | 1 +
 6 files changed, 6 insertions(+), 5 deletions(-)

On 8/8/2019 9:01 PM, rzumer@tebako.net wrote:
> From: Raphaël Zumer <rzumer@tebako.net>
> 
> Internally, this adds an EBU3213 alias to JEDEC_P22,
> and changes the name string to match ITU-T H.273.
> ---
>  libavcodec/options_table.h  | 2 +-
>  libavfilter/vf_colorspace.c | 2 +-
>  libavfilter/vf_setparams.c  | 2 +-
>  libavfilter/vf_zscale.c     | 2 +-
>  libavutil/pixdesc.c         | 2 +-
>  libavutil/pixfmt.h          | 1 +
>  6 files changed, 6 insertions(+), 5 deletions(-)

This should be split into three patches. The first one adding the enum
value to pixfmt.h, changing the string in pixdesc.c, bumping libavutil
minor version, and adding an entry to doc/APIChanges. The subject should
be something like "avutil/pixfmt: Add EBU Tech. 3213-E AVColorPrimaries
value" plus the explanation that it's an alias for Jedec P22 (Or that
one becoming an alias for the new one) after it.
Then another patch changing the libavfilter option values and bumping
libavfilter micro version, and another doing the same with libavcodec.

> 
> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> index 4a266eca16..9d82188171 100644
> --- a/libavcodec/options_table.h
> +++ b/libavcodec/options_table.h
> @@ -365,7 +365,7 @@ static const AVOption avcodec_options[] = {
>  {"smpte428_1",  "SMPTE 428-1",    0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_PRI_SMPTE428 },     INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
>  {"smpte431",    "SMPTE 431-2",    0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_PRI_SMPTE431 },     INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
>  {"smpte432",    "SMPTE 422-1",    0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_PRI_SMPTE432 },     INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> -{"jedec-p22",   "JEDEC P22",      0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_PRI_JEDEC_P22 },    INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> +{"ebu3213",     "EBU 3213-E",     0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_PRI_JEDEC_P22 },    INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
>  {"unspecified", "Unspecified",    0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_PRI_UNSPECIFIED },  INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
>  {"color_trc", "color transfer characteristics", OFFSET(color_trc), AV_OPT_TYPE_INT, {.i64 = AVCOL_TRC_UNSPECIFIED }, 1, INT_MAX, V|E|D, "color_trc_type"},
>  {"bt709",        "BT.709",           0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_TRC_BT709 },        INT_MIN, INT_MAX, V|E|D, "color_trc_type"},
> diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
> index df6efffb3d..5f22f92507 100644
> --- a/libavfilter/vf_colorspace.c
> +++ b/libavfilter/vf_colorspace.c
> @@ -968,7 +968,7 @@ static const AVOption colorspace_options[] = {
>      ENUM("smpte431",     AVCOL_PRI_SMPTE431,   "prm"),
>      ENUM("smpte432",     AVCOL_PRI_SMPTE432,   "prm"),
>      ENUM("bt2020",       AVCOL_PRI_BT2020,     "prm"),
> -    ENUM("jedec-p22",    AVCOL_PRI_JEDEC_P22,  "prm"),
> +    ENUM("ebu3213",      AVCOL_PRI_JEDEC_P22,  "prm"),
>  
>      { "trc",        "Output transfer characteristics",
>        OFFSET(user_trc),   AV_OPT_TYPE_INT, { .i64 = AVCOL_TRC_UNSPECIFIED },
> diff --git a/libavfilter/vf_setparams.c b/libavfilter/vf_setparams.c
> index fe298e5a06..80e61f851e 100644
> --- a/libavfilter/vf_setparams.c
> +++ b/libavfilter/vf_setparams.c
> @@ -74,7 +74,7 @@ static const AVOption setparams_options[] = {
>      {"smpte428",                        NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_PRI_SMPTE428},     INT_MIN, INT_MAX, FLAGS, "color_primaries"},
>      {"smpte431",                        NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_PRI_SMPTE431},     INT_MIN, INT_MAX, FLAGS, "color_primaries"},
>      {"smpte432",                        NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_PRI_SMPTE432},     INT_MIN, INT_MAX, FLAGS, "color_primaries"},
> -    {"jedec-p22",                       NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_PRI_JEDEC_P22},    INT_MIN, INT_MAX, FLAGS, "color_primaries"},
> +    {"ebu3213",                         NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_PRI_JEDEC_P22},    INT_MIN, INT_MAX, FLAGS, "color_primaries"},
>  
>      {"color_trc", "select color transfer", OFFSET(color_trc), AV_OPT_TYPE_INT, {.i64=-1}, -1, AVCOL_TRC_NB-1, FLAGS, "color_trc"},
>      {"auto", "keep the same color transfer",  0, AV_OPT_TYPE_CONST, {.i64=-1},                     INT_MIN, INT_MAX, FLAGS, "color_trc"},
> diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
> index f0309272fa..e53d7c1ae0 100644
> --- a/libavfilter/vf_zscale.c
> +++ b/libavfilter/vf_zscale.c
> @@ -788,7 +788,7 @@ static const AVOption zscale_options[] = {
>      {     "smpte428",         0,       0,                 AV_OPT_TYPE_CONST, {.i64 = ZIMG_PRIMARIES_ST428},       0, 0, FLAGS, "primaries" },
>      {     "smpte431",         0,       0,                 AV_OPT_TYPE_CONST, {.i64 = ZIMG_PRIMARIES_ST431_2},     0, 0, FLAGS, "primaries" },
>      {     "smpte432",         0,       0,                 AV_OPT_TYPE_CONST, {.i64 = ZIMG_PRIMARIES_ST432_1},     0, 0, FLAGS, "primaries" },
> -    {     "jedec-p22",        0,       0,                 AV_OPT_TYPE_CONST, {.i64 = ZIMG_PRIMARIES_EBU3213_E},   0, 0, FLAGS, "primaries" },
> +    {     "ebu3213",          0,       0,                 AV_OPT_TYPE_CONST, {.i64 = ZIMG_PRIMARIES_EBU3213_E},   0, 0, FLAGS, "primaries" },
>      { "transfer", "set transfer characteristic", OFFSET(trc), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, FLAGS, "transfer" },
>      { "t",        "set transfer characteristic", OFFSET(trc), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, FLAGS, "transfer" },
>      {     "input",            0,       0,                 AV_OPT_TYPE_CONST, {.i64 = -1},                         0, 0, FLAGS, "transfer" },

All the jedec-p22 values above can't be removed, since they are public
facing, much like the actual enum value in pixfmt.h

Just add the new ebu3213 values using AVCOL_PRI_EBU3213, and leave the
jedec-p22 ones alone.

> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
> index b97b0665b0..3b2d3b3123 100644
> --- a/libavutil/pixdesc.c
> +++ b/libavutil/pixdesc.c
> @@ -2369,7 +2369,7 @@ static const char * const color_primaries_names[AVCOL_PRI_NB] = {
>      [AVCOL_PRI_SMPTE428] = "smpte428",
>      [AVCOL_PRI_SMPTE431] = "smpte431",
>      [AVCOL_PRI_SMPTE432] = "smpte432",
> -    [AVCOL_PRI_JEDEC_P22] = "jedec-p22",
> +    [AVCOL_PRI_JEDEC_P22] = "ebu3213",

Use AVCOL_PRI_EBU3213.

>  };
>  
>  static const char * const color_transfer_names[] = {
> diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
> index 8b54c9415b..7f7b537721 100644
> --- a/libavutil/pixfmt.h
> +++ b/libavutil/pixfmt.h
> @@ -457,6 +457,7 @@ enum AVColorPrimaries {
>      AVCOL_PRI_SMPTE431    = 11, ///< SMPTE ST 431-2 (2011) / DCI P3
>      AVCOL_PRI_SMPTE432    = 12, ///< SMPTE ST 432-1 (2010) / P3 D65 / Display P3
>      AVCOL_PRI_JEDEC_P22   = 22, ///< JEDEC P22 phosphors
> +    AVCOL_PRI_EBU3213     = AVCOL_PRI_JEDEC_P22,

Maybe do it the other way around, so the doxygen comment is also changed.

AVCOL_PRI_EBU3213     = 22, ///< EBU Tech. 3213-E / JEDEC P22 phosphors
AVCOL_PRI_JEDEC_P22   = AVCOL_PRI_EBU3213,

>      AVCOL_PRI_NB                ///< Not part of ABI
>  };
>  
>

Is there a change to include the EBU primaries?

https://tech.ebu.ch/docs/tech/tech3213.pdf

White 0.313 0.329
Red 0.64 0.33
Green 0.29 0.60
Blue 0.15 0.06

as the ones currently called AVCOL_PRI_JEDEC_P22 are not those ones at
least in vf_colorspace.c

    [AVCOL_PRI_JEDEC_P22] = { WP_D65, { 0.630, 0.340, 0.295, 0.605,
0.155, 0.077 } },

Kevin


On Fri, Aug 9, 2019 at 1:48 AM James Almer <jamrial@gmail.com> wrote:
>
> On 8/8/2019 9:01 PM, rzumer@tebako.net wrote:
> > From: Raphaël Zumer <rzumer@tebako.net>
> >
> > Internally, this adds an EBU3213 alias to JEDEC_P22,
> > and changes the name string to match ITU-T H.273.
> > ---
> >  libavcodec/options_table.h  | 2 +-
> >  libavfilter/vf_colorspace.c | 2 +-
> >  libavfilter/vf_setparams.c  | 2 +-
> >  libavfilter/vf_zscale.c     | 2 +-
> >  libavutil/pixdesc.c         | 2 +-
> >  libavutil/pixfmt.h          | 1 +
> >  6 files changed, 6 insertions(+), 5 deletions(-)
>
> This should be split into three patches. The first one adding the enum
> value to pixfmt.h, changing the string in pixdesc.c, bumping libavutil
> minor version, and adding an entry to doc/APIChanges. The subject should
> be something like "avutil/pixfmt: Add EBU Tech. 3213-E AVColorPrimaries
> value" plus the explanation that it's an alias for Jedec P22 (Or that
> one becoming an alias for the new one) after it.
> Then another patch changing the libavfilter option values and bumping
> libavfilter micro version, and another doing the same with libavcodec.
>
> >
> > diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> > index 4a266eca16..9d82188171 100644
> > --- a/libavcodec/options_table.h
> > +++ b/libavcodec/options_table.h
> > @@ -365,7 +365,7 @@ static const AVOption avcodec_options[] = {
> >  {"smpte428_1",  "SMPTE 428-1",    0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_PRI_SMPTE428 },     INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> >  {"smpte431",    "SMPTE 431-2",    0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_PRI_SMPTE431 },     INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> >  {"smpte432",    "SMPTE 422-1",    0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_PRI_SMPTE432 },     INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> > -{"jedec-p22",   "JEDEC P22",      0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_PRI_JEDEC_P22 },    INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> > +{"ebu3213",     "EBU 3213-E",     0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_PRI_JEDEC_P22 },    INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> >  {"unspecified", "Unspecified",    0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_PRI_UNSPECIFIED },  INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> >  {"color_trc", "color transfer characteristics", OFFSET(color_trc), AV_OPT_TYPE_INT, {.i64 = AVCOL_TRC_UNSPECIFIED }, 1, INT_MAX, V|E|D, "color_trc_type"},
> >  {"bt709",        "BT.709",           0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_TRC_BT709 },        INT_MIN, INT_MAX, V|E|D, "color_trc_type"},
> > diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
> > index df6efffb3d..5f22f92507 100644
> > --- a/libavfilter/vf_colorspace.c
> > +++ b/libavfilter/vf_colorspace.c
> > @@ -968,7 +968,7 @@ static const AVOption colorspace_options[] = {
> >      ENUM("smpte431",     AVCOL_PRI_SMPTE431,   "prm"),
> >      ENUM("smpte432",     AVCOL_PRI_SMPTE432,   "prm"),
> >      ENUM("bt2020",       AVCOL_PRI_BT2020,     "prm"),
> > -    ENUM("jedec-p22",    AVCOL_PRI_JEDEC_P22,  "prm"),
> > +    ENUM("ebu3213",      AVCOL_PRI_JEDEC_P22,  "prm"),
> >
> >      { "trc",        "Output transfer characteristics",
> >        OFFSET(user_trc),   AV_OPT_TYPE_INT, { .i64 = AVCOL_TRC_UNSPECIFIED },
> > diff --git a/libavfilter/vf_setparams.c b/libavfilter/vf_setparams.c
> > index fe298e5a06..80e61f851e 100644
> > --- a/libavfilter/vf_setparams.c
> > +++ b/libavfilter/vf_setparams.c
> > @@ -74,7 +74,7 @@ static const AVOption setparams_options[] = {
> >      {"smpte428",                        NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_PRI_SMPTE428},     INT_MIN, INT_MAX, FLAGS, "color_primaries"},
> >      {"smpte431",                        NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_PRI_SMPTE431},     INT_MIN, INT_MAX, FLAGS, "color_primaries"},
> >      {"smpte432",                        NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_PRI_SMPTE432},     INT_MIN, INT_MAX, FLAGS, "color_primaries"},
> > -    {"jedec-p22",                       NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_PRI_JEDEC_P22},    INT_MIN, INT_MAX, FLAGS, "color_primaries"},
> > +    {"ebu3213",                         NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_PRI_JEDEC_P22},    INT_MIN, INT_MAX, FLAGS, "color_primaries"},
> >
> >      {"color_trc", "select color transfer", OFFSET(color_trc), AV_OPT_TYPE_INT, {.i64=-1}, -1, AVCOL_TRC_NB-1, FLAGS, "color_trc"},
> >      {"auto", "keep the same color transfer",  0, AV_OPT_TYPE_CONST, {.i64=-1},                     INT_MIN, INT_MAX, FLAGS, "color_trc"},
> > diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
> > index f0309272fa..e53d7c1ae0 100644
> > --- a/libavfilter/vf_zscale.c
> > +++ b/libavfilter/vf_zscale.c
> > @@ -788,7 +788,7 @@ static const AVOption zscale_options[] = {
> >      {     "smpte428",         0,       0,                 AV_OPT_TYPE_CONST, {.i64 = ZIMG_PRIMARIES_ST428},       0, 0, FLAGS, "primaries" },
> >      {     "smpte431",         0,       0,                 AV_OPT_TYPE_CONST, {.i64 = ZIMG_PRIMARIES_ST431_2},     0, 0, FLAGS, "primaries" },
> >      {     "smpte432",         0,       0,                 AV_OPT_TYPE_CONST, {.i64 = ZIMG_PRIMARIES_ST432_1},     0, 0, FLAGS, "primaries" },
> > -    {     "jedec-p22",        0,       0,                 AV_OPT_TYPE_CONST, {.i64 = ZIMG_PRIMARIES_EBU3213_E},   0, 0, FLAGS, "primaries" },
> > +    {     "ebu3213",          0,       0,                 AV_OPT_TYPE_CONST, {.i64 = ZIMG_PRIMARIES_EBU3213_E},   0, 0, FLAGS, "primaries" },
> >      { "transfer", "set transfer characteristic", OFFSET(trc), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, FLAGS, "transfer" },
> >      { "t",        "set transfer characteristic", OFFSET(trc), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, FLAGS, "transfer" },
> >      {     "input",            0,       0,                 AV_OPT_TYPE_CONST, {.i64 = -1},                         0, 0, FLAGS, "transfer" },
>
> All the jedec-p22 values above can't be removed, since they are public
> facing, much like the actual enum value in pixfmt.h
>
> Just add the new ebu3213 values using AVCOL_PRI_EBU3213, and leave the
> jedec-p22 ones alone.
>
> > diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
> > index b97b0665b0..3b2d3b3123 100644
> > --- a/libavutil/pixdesc.c
> > +++ b/libavutil/pixdesc.c
> > @@ -2369,7 +2369,7 @@ static const char * const color_primaries_names[AVCOL_PRI_NB] = {
> >      [AVCOL_PRI_SMPTE428] = "smpte428",
> >      [AVCOL_PRI_SMPTE431] = "smpte431",
> >      [AVCOL_PRI_SMPTE432] = "smpte432",
> > -    [AVCOL_PRI_JEDEC_P22] = "jedec-p22",
> > +    [AVCOL_PRI_JEDEC_P22] = "ebu3213",
>
> Use AVCOL_PRI_EBU3213.
>
> >  };
> >
> >  static const char * const color_transfer_names[] = {
> > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
> > index 8b54c9415b..7f7b537721 100644
> > --- a/libavutil/pixfmt.h
> > +++ b/libavutil/pixfmt.h
> > @@ -457,6 +457,7 @@ enum AVColorPrimaries {
> >      AVCOL_PRI_SMPTE431    = 11, ///< SMPTE ST 431-2 (2011) / DCI P3
> >      AVCOL_PRI_SMPTE432    = 12, ///< SMPTE ST 432-1 (2010) / P3 D65 / Display P3
> >      AVCOL_PRI_JEDEC_P22   = 22, ///< JEDEC P22 phosphors
> > +    AVCOL_PRI_EBU3213     = AVCOL_PRI_JEDEC_P22,
>
> Maybe do it the other way around, so the doxygen comment is also changed.
>
> AVCOL_PRI_EBU3213     = 22, ///< EBU Tech. 3213-E / JEDEC P22 phosphors
> AVCOL_PRI_JEDEC_P22   = AVCOL_PRI_EBU3213,
>
> >      AVCOL_PRI_NB                ///< Not part of ABI
> >  };
> >
> >
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

On Fri, Aug 9, 2019 at 12:32 PM Kevin Wheatley
<kevin.j.wheatley@gmail.com> wrote:
>
> Is there a change to include the EBU primaries?
>
> https://tech.ebu.ch/docs/tech/tech3213.pdf
>
> White 0.313 0.329
> Red 0.64 0.33
> Green 0.29 0.60
> Blue 0.15 0.06
>
> as the ones currently called AVCOL_PRI_JEDEC_P22 are not those ones at
> least in vf_colorspace.c
>
>     [AVCOL_PRI_JEDEC_P22] = { WP_D65, { 0.630, 0.340, 0.295, 0.605,
> 0.155, 0.077 } },
>

The enum and our values are aligned to the H.273 / ISO/IEC 23001-8
standards, which documents this entry to correspond to the primaries
in use by vf_colorspace

- Hendrik

On Fri, Aug 9, 2019 at 12:40 PM Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> The enum and our values are aligned to the H.273 / ISO/IEC 23001-8
> standards, which documents this entry to correspond to the primaries
> in use by vf_colorspace

That makes sense, although I'm now interested to find out where those
numbers came from as they didn't come out of the EBU document, guess
I'll have to ask next time I bump into somebody from the ITU or EBU,
the EBU numbers do come with allowable tolerances, but the blue
co-ordinate appears to fall outside of that.

Thanks

Kevin