[vlc-devel] [PATCH] include: add enums for colorimetry

[Vittorio Giovara]

On Sat, Apr 16, 2016 at 10:21 AM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> Le 2016-04-16 16:10, Vittorio Giovara a écrit :
>>
>> On Sat, Apr 16, 2016 at 7:42 AM, Rémi Denis-Courmont <remi at remlab.net>
>> wrote:
>>>
>>> ---
>>>  include/vlc_es.h     | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>>  include/vlc_plugin.h |  4 ++--
>>>  2 files changed, 47 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/vlc_es.h b/include/vlc_es.h
>>> index 97cdb78..7e141a4 100644
>>> --- a/include/vlc_es.h
>>> +++ b/include/vlc_es.h
>>> @@ -193,6 +193,48 @@ typedef enum video_transform_t
>>>  } video_transform_t;
>>>
>>>  /**
>>> + * Video color primaries (a.k.a. chromacities)
>>> + */
>>> +typedef enum video_color_primaries_t
>>> +{
>>> +    COLOR_PRIMARIES_UNDEF,
>>> +    COLOR_PRIMARIES_BT601_525,
>>> +    COLOR_PRIMARIES_BT601_625,
>>> +    COLOR_PRIMARIES_BT709,
>>> +    COLOR_PRIMARIES_BT2020,
>>> +    COLOR_PRIMARIES_DCI_P3,
>>> +#define COLOR_SRGB COLOR_BT709
>>> +} video_color_primaries_t;
>>> +
>>> +/**
>>> + * Video transfer functions
>>> + */
>>> +typedef enum video_transfer_func_t
>>> +{
>>> +    TRANSFER_FUNC_UNDEF,
>>> +    TRANSFER_FUNC_LINEAR,
>>> +    TRANSFER_FUNC_SRGB /*< Gamma 2.2 */,
>>> +    TRANSFER_FUNC_BT709,
>>> +#define TRANSFER_FUNC_BT2020 TRANSFER_FUNC_BT709
>>> +} video_transfer_func_t;
>>> +
>>> +/**
>>> + * Video color space (i.e. YCbCr matrices)
>>> + */
>>> +typedef enum video_color_space_t
>>> +{
>>> +    COLOR_SPACE_UNDEF,
>>> +    COLOR_SPACE_BT601_LIMITED,
>>> +#define COLOR_SPACE_BT601 COLOR_SPACE_BT601_LIMITED
>>> +    COLOR_SPACE_BT601_FULL,
>>> +    COLOR_SPACE_BT709_LIMITED,
>>> +#define COLOR_SPACE_BT709 COLOR_SPACE_BT709_LIMITED
>>> +    COLOR_SPACE_BT709_FULL,
>>> +    COLOR_SPACE_BT2020_LIMITED,
>>> +#define COLOR_SPACE_BT2020 COLOR_SPACE_BT2020_LIMITED
>>> +} video_color_space_t;
>>
>>
>> why multiple values for the same color space instead of a separate
>> enum (with only limited/full) or an extra bool (0=liited, 1=full)?
>
>
> In practive, limited and full affect the coefficients of the same color
> space matrix. Also separating them introduces impossible cases (AFAIK, full
> range BT.2020 does not exist yet).

To my knowledge, limited/full only mandate how many quantization
levels you can have to represent one of the color components of a
pixel, it has nothing to do with the matrix it is applied on. AFAIU
neither bt709 has full range, but samples exists anyway.

Also API-wise it's filling the code with

+        return b_full_range ? COLOR_SPACE_BT601_FULL
+                            : COLOR_SPACE_BT601_LIMITED;

every time you fill in color space information. I personally believe
that just returning the color space and the range value separately is
simpler and cleaner.
-- 
Vittorio