[vlc-devel] [PATCH] vlc_es: add mapping for all known colorspaces

Jean-Baptiste Kempf jb at videolan.org
Mon Dec 26 22:08:15 CET 2016


Hello

On Mon, 26 Dec 2016, at 16:27, Francois Cartegnie wrote:
>  {
>      COLOR_PRIMARIES_UNDEF,
> -    COLOR_PRIMARIES_BT601_525,
> -    COLOR_PRIMARIES_BT601_625,
>      COLOR_PRIMARIES_BT709,
> +    COLOR_PRIMARIES_BT470_M,
> +    COLOR_PRIMARIES_BT470_BG,
> +    COLOR_PRIMARIES_BT601_525,
> +    // COLOR_PRIMARIES_SMTPE_240
>      COLOR_PRIMARIES_BT2020,
>      COLOR_PRIMARIES_DCI_P3,
> -    COLOR_PRIMARIES_FCC1953,
> +    COLOR_PRIMARIES_SMPTE_ST428,
> +    COLOR_PRIMARIES_SMPTE_431,
> +    COLOR_PRIMARIES_SMPTE_432,
>  #define COLOR_PRIMARIES_SRGB            COLOR_PRIMARIES_BT709
>  #define COLOR_PRIMARIES_SMTPE_170       COLOR_PRIMARIES_BT601_525
>  #define COLOR_PRIMARIES_SMTPE_240       COLOR_PRIMARIES_BT601_525 /*
>  Only differs from 1e10-4 in white Y */
>  #define COLOR_PRIMARIES_SMTPE_RP145     COLOR_PRIMARIES_BT601_525
>  #define COLOR_PRIMARIES_EBU_3213        COLOR_PRIMARIES_BT601_625
> -#define COLOR_PRIMARIES_BT470_BG        COLOR_PRIMARIES_BT601_625
> -#define COLOR_PRIMARIES_BT470_M         COLOR_PRIMARIES_FCC1953
> +#define COLOR_PRIMARIES_BT601_625       COLOR_PRIMARIES_BT470_BG
> +#define COLOR_PRIMARIES_FCC1953         COLOR_PRIMARIES_BT470_M
>  } video_color_primaries_t;

I'm not sure about those changes (the one removing the defines).
About the 431/432, do those really exist in real life?

> @@ -254,18 +258,28 @@ typedef enum video_color_primaries_t
>  typedef enum video_transfer_func_t
>  {
>      TRANSFER_FUNC_UNDEF,
> -    TRANSFER_FUNC_LINEAR,
> -    TRANSFER_FUNC_SRGB /*< Gamma 2.2 */,
> -    TRANSFER_FUNC_BT470_BG,
> -    TRANSFER_FUNC_BT470_M,
>      TRANSFER_FUNC_BT709,
> -    TRANSFER_FUNC_SMPTE_ST2084,
> +    TRANSFER_FUNC_BT470_M,  /*< Gamma 2.2 */
> +    TRANSFER_FUNC_BT470_BG, /*< Gamma 2.8 */
> +    TRANSFER_FUNC_BT601,
>      TRANSFER_FUNC_SMPTE_240,
> -#define TRANSFER_FUNC_BT2020            TRANSFER_FUNC_BT709
> -#define TRANSFER_FUNC_SMPTE_170         TRANSFER_FUNC_BT709
> +    TRANSFER_FUNC_LINEAR,
> +    TRANSFER_FUNC_LOG,
> +    TRANSFER_FUNC_LOG_SQRT,
> +    TRANSFER_FUNC_IEC61966_2_4,
> +    TRANSFER_FUNC_BT1361,
> +    TRANSFER_FUNC_IEC61966_2_1,
> +    TRANSFER_FUNC_BT2020_V14,
> +    TRANSFER_FUNC_BT2020_V15,
> +    TRANSFER_FUNC_SMPTE_ST2084,
> +    TRANSFER_FUNC_SMPTE_ST428,
> +    TRANSFER_FUNC_ARIB_STD_B67,
> +#define TRANSFER_FUNC_SRGB              TRANSFER_FUNC_BT470_M
> +#define TRANSFER_FUNC_SMPTE_170         TRANSFER_FUNC_BT601
>  #define TRANSFER_FUNC_SMPTE_274         TRANSFER_FUNC_BT709
>  #define TRANSFER_FUNC_SMPTE_293         TRANSFER_FUNC_BT709
>  #define TRANSFER_FUNC_SMPTE_296         TRANSFER_FUNC_BT709
> +#define TRANSFER_FUNC_BT2020            TRANSFER_FUNC_BT2020_V14
>  } video_transfer_func_t;


We clearly don't need all those, notably the LOG, LOG_SQRT and such.
However STD_B67 is just HLG and we MUST have it, since it's one of the
standard for HDR, with HDR10 and DolbyVision.

>  typedef enum video_color_space_t
>  {
>      COLOR_SPACE_UNDEF,
> -    COLOR_SPACE_BT601,
> +    COLOR_SPACE_RGB,
>      COLOR_SPACE_BT709,
> -    COLOR_SPACE_BT2020,
> -#define COLOR_SPACE_SRGB      COLOR_SPACE_BT709
> -#define COLOR_SPACE_SMPTE_170 COLOR_SPACE_BT601
> -#define COLOR_SPACE_SMPTE_240 COLOR_SPACE_SMPTE_170
> +    COLOR_SPACE_FCC,
> +    COLOR_SPACE_BT470BG,
> +    COLOR_SPACE_BT601_525,
> +    COLOR_SPACE_SMPTE_240M,
> +    COLOR_SPACE_YCOCG,
> +    COLOR_SPACE_BT2020_NCL,
> +    COLOR_SPACE_BT2020_CL,
> +    COLOR_SPACE_SMPTE_2085,
> +#define COLOR_SPACE_SRGB      COLOR_SPACE_RGB
> +#define COLOR_SPACE_BT601     COLOR_SPACE_BT601_525
> +#define COLOR_SPACE_SMPTE_170 COLOR_SPACE_BT601_525
> +#define COLOR_SPACE_BT2020    COLOR_SPACE_BT2020_NCL
> +#define COLOR_SPACE_YCGCO     COLOR_SPACE_YCOCG
>  } video_color_space_t;

Same remark. I doubt anyone really cares about YCGCO and some 2020 are
like totally unused.

> -            p_fmt->space = COLOR_SPACE_BT601;
> +            p_fmt->space = COLOR_SPACE_BT601_525;

I don't get the reason of this change, 601 almost always means 601_525.

> --- a/modules/codec/avcodec/chroma.c
> +++ b/modules/codec/avcodec/chroma.c
> @@ -218,3 +218,101 @@ int FindFfmpegChroma( vlc_fourcc_t fourcc )
>              return chroma_table[i].i_chroma_id;
>      return AV_PIX_FMT_NONE;
>  }
> +
> +#define RETURN_TABLE_LOOKUP( table, var, matchmbr, retmbr )\
> +    for( unsigned i=0; i<ARRAY_SIZE(table); i++ )\
> +    {\
> +        if( table[i].matchmbr == var )\
> +            return table[i].retmbr;\
> +    }\
> +    return table[0].retmbr;
> +
> +static const struct
> +{
> +    enum video_color_primaries_t vlc;
> +    enum AVColorPrimaries ff;
> +} color_primaries_table[] = {
> +    { COLOR_PRIMARIES_UNDEF,        AVCOL_PRI_UNSPECIFIED   },
> +    { COLOR_PRIMARIES_UNDEF,        AVCOL_PRI_NB            },
> +    { COLOR_PRIMARIES_BT709,        AVCOL_PRI_BT709         },
> +    { COLOR_PRIMARIES_BT470_M,      AVCOL_PRI_BT470M        },
> +    { COLOR_PRIMARIES_BT470_BG,     AVCOL_PRI_BT470BG       },
> +    { COLOR_PRIMARIES_BT601_525,    AVCOL_PRI_SMPTE170M     },
> +    { COLOR_PRIMARIES_BT2020,       AVCOL_PRI_BT2020        },
> +    { COLOR_PRIMARIES_DCI_P3,       AVCOL_PRI_NB            },
> +    { COLOR_PRIMARIES_SMPTE_ST428,  AVCOL_PRI_SMPTEST428_1  },
> +    { COLOR_PRIMARIES_SMPTE_431,    AVCOL_PRI_SMPTE431      },
> +    { COLOR_PRIMARIES_SMPTE_432,    AVCOL_PRI_SMPTE432      },
> +};
> +
> +video_color_primaries_t GetVlcColorPrimaries( enum AVColorPrimaries ff )
> +{
> +    RETURN_TABLE_LOOKUP(color_primaries_table, ff, ff, vlc);
> +}
> +
> +enum AVColorPrimaries GetFfColorPrimaries( video_color_primaries_t vlc )
> +{
> +    RETURN_TABLE_LOOKUP(color_primaries_table, vlc, vlc, ff);
> +}
> +
> +static const struct
> +{
> +    enum video_transfer_func_t vlc;
> +    enum AVColorTransferCharacteristic ff;
> +} color_transfert_table[] = {
> +    { TRANSFER_FUNC_UNDEF,         AVCOL_TRC_UNSPECIFIED   },
> +    { TRANSFER_FUNC_BT709,         AVCOL_TRC_BT709         },
> +    { TRANSFER_FUNC_BT470_M,       AVCOL_TRC_GAMMA22       },
> +    { TRANSFER_FUNC_BT470_BG,      AVCOL_TRC_GAMMA28       },
> +    { TRANSFER_FUNC_BT601,         AVCOL_TRC_SMPTE170M     },
> +    { TRANSFER_FUNC_SMPTE_240,     AVCOL_TRC_SMPTE240M     },
> +    { TRANSFER_FUNC_LINEAR,        AVCOL_TRC_LINEAR        },
> +    { TRANSFER_FUNC_LOG,           AVCOL_TRC_LOG           },
> +    { TRANSFER_FUNC_LOG_SQRT,      AVCOL_TRC_LOG_SQRT      },
> +    { TRANSFER_FUNC_IEC61966_2_4,  AVCOL_TRC_IEC61966_2_4  },
> +    { TRANSFER_FUNC_BT1361,        AVCOL_TRC_BT1361_ECG    },
> +    { TRANSFER_FUNC_IEC61966_2_1,  AVCOL_TRC_IEC61966_2_1  },
> +    { TRANSFER_FUNC_BT2020_V14,    AVCOL_TRC_BT2020_10     },
> +    { TRANSFER_FUNC_BT2020_V15,    AVCOL_TRC_BT2020_12     },
> +    { TRANSFER_FUNC_SMPTE_ST2084,  AVCOL_TRC_SMPTEST2084   },
> +    { TRANSFER_FUNC_SMPTE_ST428,   AVCOL_TRC_SMPTEST428_1  },
> +    { TRANSFER_FUNC_ARIB_STD_B67,  AVCOL_TRC_ARIB_STD_B67  },
> +};
> +
> +video_transfer_func_t GetVlcColorTransfert( enum
> AVColorTransferCharacteristic ff )
> +{
> +    RETURN_TABLE_LOOKUP(color_transfert_table, ff, ff, vlc);
> +}
> +
> +enum AVColorTransferCharacteristic GetFfColorTransfert(
> video_transfer_func_t vlc )
> +{
> +    RETURN_TABLE_LOOKUP(color_transfert_table, vlc, vlc, ff);
> +}
> +
> +static const struct
> +{
> +    enum video_color_space_t vlc;
> +    enum AVColorSpace ff;
> +} color_spaces_table[] = {
> +    { COLOR_SPACE_UNDEF,            AVCOL_SPC_UNSPECIFIED   },
> +    { COLOR_SPACE_RGB,              AVCOL_SPC_RGB           },
> +    { COLOR_SPACE_BT709,            AVCOL_SPC_BT709         },
> +    { COLOR_SPACE_FCC,              AVCOL_SPC_FCC           },
> +    { COLOR_SPACE_BT470BG,          AVCOL_SPC_BT470BG       },
> +    { COLOR_SPACE_BT601_525,        AVCOL_SPC_SMPTE170M     },
> +    { COLOR_SPACE_SMPTE_240M,       AVCOL_SPC_SMPTE240M     },
> +    { COLOR_SPACE_YCOCG,            AVCOL_SPC_YCOCG         },
> +    { COLOR_SPACE_BT2020_NCL,       AVCOL_SPC_BT2020_NCL    },
> +    { COLOR_SPACE_BT2020_CL,        AVCOL_SPC_BT2020_CL     },
> +    { COLOR_SPACE_SMPTE_2085,       AVCOL_SPC_SMPTE2085     },
> +};
> +
> +video_color_space_t GetVlcColorSpace( enum AVColorSpace ff )
> +{
> +    RETURN_TABLE_LOOKUP(color_spaces_table, ff, ff, vlc);
> +}
> +
> +enum AVColorSpace GetFfColorSpace( video_color_space_t vlc )
> +{
> +    RETURN_TABLE_LOOKUP(color_spaces_table, vlc, vlc, ff);
> +}
> diff --git a/modules/codec/avcodec/chroma.h
> b/modules/codec/avcodec/chroma.h
> index 99dc1d6..857d9ab 100644
> --- a/modules/codec/avcodec/chroma.h
> +++ b/modules/codec/avcodec/chroma.h
> @@ -32,4 +32,13 @@ int GetFfmpegChroma( int *i_ffmpeg_chroma, const
> video_format_t *fmt );
>  vlc_fourcc_t FindVlcChroma( int );
>  int GetVlcChroma( video_format_t *fmt, int i_ffmpeg_chroma );
>  
> +video_color_primaries_t GetVlcColorPrimaries( enum AVColorPrimaries ff
> );
> +enum AVColorPrimaries GetFfColorPrimaries( video_color_primaries_t vlc
> );
> +
> +video_transfer_func_t GetVlcColorTransfert( enum
> AVColorTransferCharacteristic ff );
> +enum AVColorTransferCharacteristic GetFfColorTransfert(
> video_transfer_func_t vlc );
> +
> +video_color_space_t GetVlcColorSpace( enum AVColorSpace ff );
> +enum AVColorSpace GetFfColorSpace( video_color_space_t vlc );
> +
>  #endif


Those are a good idea, but should be in a different patch.


> +static const struct
> +{
> +    enum hxxx_colour_primaries i_hxxx_val;
> +    video_color_primaries_t i_vlc_val;
> +} hxxx_color_primaries_mapping[] = {
> +    { HXXX_PRIMARIES_BT709,         COLOR_PRIMARIES_BT709       },
> +    { HXXX_PRIMARIES_BT470M,        COLOR_PRIMARIES_BT470_M     },
> +    { HXXX_PRIMARIES_BT470BG,       COLOR_PRIMARIES_BT470_BG    },
> +    { HXXX_PRIMARIES_BT601_525,     COLOR_PRIMARIES_BT601_625   },
> +    { HXXX_PRIMARIES_SMTPE_240M,    COLOR_PRIMARIES_SMTPE_240   },
> +    { HXXX_PRIMARIES_GENERIC_FILM,  COLOR_PRIMARIES_UNDEF       },
> +    { HXXX_PRIMARIES_BT2020,        COLOR_PRIMARIES_BT2020      },
> +    { HXXX_PRIMARIES_SMPTE_ST_428,  COLOR_PRIMARIES_SMPTE_ST428 },
> +};

As far as I'm concerned, this list is a very good summary of what we
should support, with a bemol for 428.
240M should die, though.

> +} hxxx_transfer_characteristics_mapping[] = {
> +    { HXXX_TRANSFER_BT709,         TRANSFER_FUNC_BT709       },
> +    { HXXX_TRANSFER_BT470M,        TRANSFER_FUNC_BT470_M     },
> +    { HXXX_TRANSFER_BT470BG,       TRANSFER_FUNC_BT470_BG    },
> +    { HXXX_TRANSFER_BT601_525,     TRANSFER_FUNC_BT601       },
> +    { HXXX_TRANSFER_SMTPE_240M,    TRANSFER_FUNC_SMPTE_240   },
> +    { HXXX_TRANSFER_LINEAR,        TRANSFER_FUNC_LINEAR      },
> +    { HXXX_TRANSFER_LOG,           TRANSFER_FUNC_LOG         },
> +    { HXXX_TRANSFER_LOG_SQRT,      TRANSFER_FUNC_LOG_SQRT    },
> +    { HXXX_TRANSFER_IEC61966_2_4,  TRANSFER_FUNC_IEC61966_2_4},
> +    { HXXX_TRANSFER_BT1361,        TRANSFER_FUNC_BT1361      },
> +    { HXXX_TRANSFER_IEC61966_2_1,  TRANSFER_FUNC_IEC61966_2_1},
> +    { HXXX_TRANSFER_BT2020_V14,    TRANSFER_FUNC_BT2020_V14  },
> +    { HXXX_TRANSFER_BT2020_V15,    TRANSFER_FUNC_BT2020_V15  },
> +    { HXXX_TRANSFER_SMPTE_ST_2084, TRANSFER_FUNC_SMPTE_ST2084},
> +    { HXXX_TRANSFER_SMPTE_ST_428,  TRANSFER_FUNC_SMPTE_ST428 },
> +};

I doubt most of those are used, tbh.

> +} hxxx_matrix_coeffs_mapping[] = {
> +    { HXXX_MATRIX_IDENTITY,      COLOR_SPACE_RGB        },
> +    { HXXX_MATRIX_BT709,         COLOR_SPACE_BT709      },
> +    { HXXX_MATRIX_FCC,           COLOR_SPACE_FCC        },
> +    { HXXX_MATRIX_BT470BG,       COLOR_SPACE_BT470BG    },
> +    { HXXX_MATRIX_BT601_525,     COLOR_SPACE_BT601_525  },
> +    { HXXX_MATRIX_SMTPE_240M,    COLOR_SPACE_SMPTE_240M },
> +    { HXXX_MATRIX_YCGCO,         COLOR_SPACE_YCGCO      },
> +    { HXXX_MATRIX_BT2020_NCL,    COLOR_SPACE_BT2020_NCL },
> +    { HXXX_MATRIX_BT2020_CL,     COLOR_SPACE_BT2020_CL  },
> +};

Same remark as before on YCGCO and FCC

The rest looks good.

-- 
Jean-Baptiste Kempf -  President
+33 672 704 734


More information about the vlc-devel mailing list