[FFmpeg-devel] [PATCH 1/2] lavu: Add JEDEC P22 color primaries

[Andreas Cadhalpun]

On 30.11.2016 19:16, Vittorio Giovara wrote:
> Signed-off-by: Vittorio Giovara <vittorio.giovara at gmail.com>
> ---
> Please CC.
> Vittorio
> 
>  libavutil/pixdesc.c | 1 +
>  libavutil/pixfmt.h  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
> index 3b9c45d..04eab0b 100644
> --- a/libavutil/pixdesc.c
> +++ b/libavutil/pixdesc.c
> @@ -2183,6 +2183,7 @@ static const char *color_primaries_names[AVCOL_PRI_NB] = {
>      [AVCOL_PRI_SMPTE428] = "smpte428",
>      [AVCOL_PRI_SMPTE431] = "smpte431",
>      [AVCOL_PRI_SMPTE432] = "smpte432",
> +    [AVCOL_PRI_JEDEC_P22] = "jedec-p22",
>  };
>  
>  static const char *color_transfer_names[] = {
> diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
> index dfb1b11..d6c5a57 100644
> --- a/libavutil/pixfmt.h
> +++ b/libavutil/pixfmt.h
> @@ -413,6 +413,7 @@ enum AVColorPrimaries {
>      AVCOL_PRI_SMPTEST428_1 = AVCOL_PRI_SMPTE428,
>      AVCOL_PRI_SMPTE431    = 11, ///< SMPTE ST 431-2 (2011)
>      AVCOL_PRI_SMPTE432    = 12, ///< SMPTE ST 432-1 D65 (2010)
> +    AVCOL_PRI_JEDEC_P22   = 22, ///< JEDEC P22 phosphors
>      AVCOL_PRI_NB                ///< Not part of ABI
>  };

You can't just add a gap like that.
The current code assumes that the numbers are consecutive, like e.g. the
naming of AVCOL_PRI_NB suggests.
For example AVCOL_PRI_NB is used in libavcodec/options_table.h to validate
the color_primaries option, but after this patch would accept values like 15.
Also I think av_color_primaries_name would just return uninitialized memory
for such a value.

If you really need to add the gap here, you'd have to carefully look at
every occurrence of AVCOL_PRI_NB in the code base and make sure the
code still works with a gap here.

Best regards,
Andreas