[FFmpeg-devel] [PATCH] avcodec/dpxenc: stop hardcoding color trc/primaries

[Harry Mallon]

> On 7 Oct 2020, at 22:02, Paul B Mahol <onemda at gmail.com> wrote:
> 
> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> ---
> libavcodec/dpxenc.c | 36 ++++++++++++++++++++++++++++++++++--
> tests/ref/lavf/dpx  |  2 +-
> 2 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/dpxenc.c b/libavcodec/dpxenc.c
> index a5960334d5..56840a8d33 100644
> --- a/libavcodec/dpxenc.c
> +++ b/libavcodec/dpxenc.c
> @@ -173,6 +173,38 @@ static void encode_gbrp12(AVCodecContext *avctx, const AVFrame *pic, uint16_t *d
>     }
> }
> 
> +static int get_dpx_pri(int color_pri)
> +{
> +    switch (color_pri) {
> +    case AVCOL_PRI_BT709:
> +        return 6;
> +    case AVCOL_PRI_SMPTE240M:
> +    case AVCOL_PRI_SMPTE170M:
> +        return 9;

I think perhaps this should be 8 (ITU 601 525), rather than 9 (Composite video SMPTE 170M), but I am not sure?

> +    case AVCOL_PRI_BT470BG:
> +        return 10;

Perhaps this should be 7 (ITU 601 625), rather than 10 (ITU 624-4 Composite video PAL), again not sure which is most widely used?

> +    default:
> +        return 0;
> +    }
> +}

In DPX files containing colour difference information the colorspace would also be keyed off the value returned from this function. Perhaps it should be taken into account here (for files containing YCbCr)?

> +
> +static int get_dpx_trc(int color_trc)
> +{
> +    switch (color_trc) {
> +    case AVCOL_TRC_LINEAR:
> +        return 2;
> +    case AVCOL_TRC_BT709:
> +        return 6;
> +    case AVCOL_TRC_SMPTE240M:
> +    case AVCOL_TRC_SMPTE170M:
> +        return 9;

This value could be 7, 8 or 9. From my reading of the spec the best might be to take colour_primaries into account and do something like:

if (AVCOL_PRI_BT470BG) return 7 (ITU 601 625)
else return 8 (ITU 601 525)

> +    case AVCOL_TRC_GAMMA22:
> +        return 10;

10 is ITU 624-4 Composite video PAL, which says it has gamma = 2.8 (AVCOL_TRC_GAMMA28). https://www.itu.int/dms_pub/itu-r/opb/rep/R-REP-BT.624-4-1990-PDF-E.pdf

> +    default:
> +        return 0;
> +    }
> +}
> +
> 
> [..]
> 

Best,
Harry