[FFmpeg-devel] [PATCH 2/3] avformat/mxfenc: Write Mastering Display Colour Volume to MXF

[Marton Balint]

On Mon, 31 Aug 2020, Harry Mallon wrote:

> Described in Annex B SMPTE ST 2067-21:2020
>
> Signed-off-by: Harry Mallon <harry.mallon at codex.online>
> ---
> libavformat/mxfenc.c | 56 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index e495b5ba0e..fe1ecb6705 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -44,6 +44,7 @@
> #include "libavutil/random_seed.h"
> #include "libavutil/timecode.h"
> #include "libavutil/avassert.h"
> +#include "libavutil/mastering_display_metadata.h"
> #include "libavutil/pixdesc.h"
> #include "libavutil/time_internal.h"
> #include "libavcodec/bytestream.h"
> @@ -421,6 +422,13 @@ static const MXFLocalTagPair mxf_user_comments_local_tag[] = {
>     { 0x5003, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x03,0x02,0x01,0x02,0x0A,0x01,0x00,0x00}}, /* Value */
> };
> 
> +static const MXFLocalTagPair mxf_mastering_display_local_tags[] = {
> +    { 0x8201, {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x20,0x04,0x01,0x01,0x01,0x00,0x00}}, /* Mastering Display Primaries */
> +    { 0x8202, {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x20,0x04,0x01,0x01,0x02,0x00,0x00}}, /* Mastering Display White Point Chromaticity */
> +    { 0x8203, {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x20,0x04,0x01,0x01,0x03,0x00,0x00}}, /* Mastering Display Maximum Luminance */
> +    { 0x8204, {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x20,0x04,0x01,0x01,0x04,0x00,0x00}}, /* Mastering Display Minimum Luminance */

Ain't these collide with AVC subdescriptor local tags?

> +};
> +
> static void mxf_write_uuid(AVIOContext *pb, enum MXFMetadataSetType type, int value)
> {
>     avio_write(pb, uuid_base, 12);
> @@ -510,6 +518,7 @@ static void mxf_write_primer_pack(AVFormatContext *s)
>     AVIOContext *pb = s->pb;
>     int local_tag_number, i = 0;
>     int avc_tags_count = 0;
> +    int mastering_tags_count = 0;
>
>     local_tag_number = FF_ARRAY_ELEMS(mxf_local_tag_batch);
>     local_tag_number += mxf->store_user_comments * FF_ARRAY_ELEMS(mxf_user_comments_local_tag);
> @@ -522,6 +531,15 @@ static void mxf_write_primer_pack(AVFormatContext *s)
>         }
>     }
> 
> +    for (i = 0; i < s->nb_streams; i++) {
> +        uint8_t *side_data = av_stream_get_side_data(s->streams[i], AV_PKT_DATA_MASTERING_DISPLAY_METADATA, NULL);

You can get rid of the side_data temporary variable and use the function 
call directly in the if condition.

> +        if (side_data) {
> +            mastering_tags_count = FF_ARRAY_ELEMS(mxf_mastering_display_local_tags);
> +            local_tag_number += mastering_tags_count;
> +            break;
> +        }
> +    }

Push this into the previous loop. Only increase local_tag_number out of 
the loop, this way you can get rid of the break. (Similar change might 
make sense for the existing avc_tags_count code as well, because it is 
super confusing that it increases local_tag_count for every stream...)

> +
>     avio_write(pb, primer_pack_key, 16);
>     klv_encode_ber_length(pb, local_tag_number * 18 + 8);
> 
> @@ -539,6 +557,8 @@ static void mxf_write_primer_pack(AVFormatContext *s)
>         }
>     if (avc_tags_count > 0)
>         mxf_write_local_tags(pb, mxf_avc_subdescriptor_local_tags, avc_tags_count);
> +    if (mastering_tags_count > 0)
> +        mxf_write_local_tags(pb, mxf_mastering_display_local_tags, mastering_tags_count);
> }
> 
> static void mxf_write_local_tag(AVIOContext *pb, int size, int tag)
> @@ -1048,6 +1068,11 @@ static const UID mxf_generic_sound_descriptor_key = { 0x06,0x0E,0x2B,0x34,0x02,0
> 
> static const UID mxf_avc_subdescriptor_key = { 0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x6E,0x00 };
> 
> +static inline int64_t rescale_mastering(AVRational q, int b)
> +{
> +    return av_rescale(q.num, b, q.den);

Might make sense to av_clip_uint16 the result and change return type 
accordingly?

> +}
> +
> static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID key)
> {
>     MXFStreamContext *sc = st->priv_data;
> @@ -1060,6 +1085,7 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
>     const MXFCodecUL *color_trc_ul;
>     const MXFCodecUL *color_space_ul;
>     int64_t pos = mxf_write_generic_desc(s, st, key);
> +    uint8_t *side_data;
>
>     color_primaries_ul = mxf_get_codec_ul_by_id(ff_mxf_color_primaries_uls, st->codecpar->color_primaries);
>     color_trc_ul       = mxf_get_codec_ul_by_id(ff_mxf_color_trc_uls, st->codecpar->color_trc);
> @@ -1228,6 +1254,36 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
>     mxf_write_local_tag(pb, 16, 0x3201);
>     avio_write(pb, *sc->codec_ul, 16);
> 
> +    // Mastering Display metadata
> +    side_data = av_stream_get_side_data(st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA, NULL);
> +    if (side_data) {
> +        const AVMasteringDisplayMetadata *metadata = (const AVMasteringDisplayMetadata*)side_data;
> +        const int chroma_den = 50000;
> +        const int luma_den = 10000;
> +        if (metadata->has_primaries) {
> +            mxf_write_local_tag(pb, 12, 0x8201);
> +            avio_wb16(pb, rescale_mastering(metadata->display_primaries[0][0], chroma_den));
> +            avio_wb16(pb, rescale_mastering(metadata->display_primaries[0][1], chroma_den));
> +            avio_wb16(pb, rescale_mastering(metadata->display_primaries[1][0], chroma_den));
> +            avio_wb16(pb, rescale_mastering(metadata->display_primaries[1][1], chroma_den));
> +            avio_wb16(pb, rescale_mastering(metadata->display_primaries[2][0], chroma_den));
> +            avio_wb16(pb, rescale_mastering(metadata->display_primaries[2][1], chroma_den));
> +            mxf_write_local_tag(pb, 4, 0x8202);
> +            avio_wb16(pb, rescale_mastering(metadata->white_point[0], chroma_den));
> +            avio_wb16(pb, rescale_mastering(metadata->white_point[1], chroma_den));
> +        } else {
> +            av_log(NULL, AV_LOG_VERBOSE, "Not writing mastering display primaries. Missing data.\n");
> +        }
> +        if (metadata->has_luminance) {
> +            mxf_write_local_tag(pb, 4, 0x8203);
> +            avio_wb32(pb, rescale_mastering(metadata->max_luminance, luma_den));
> +            mxf_write_local_tag(pb, 4, 0x8204);
> +            avio_wb32(pb, rescale_mastering(metadata->min_luminance, luma_den));
> +        } else {
> +            av_log(NULL, AV_LOG_VERBOSE, "Not writing mastering display luminances. Missing data.\n");
> +        }
> +    }
> +

Thanks,
Marton