| Message ID | 20200831190756.80515-3-harry.mallon@codex.online |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avformat/mxfdec: Read Mastering Display Colour Volume from MXF | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
mån 2020-08-31 klockan 20:07 +0100 skrev Harry Mallon: > * As embedded by Apple Compressor This needs a sample since it isn't part of any official spec, so that we can have a test for this. > + if (IS_KLV_KEY(uid, mxf_coll_apple_max_cll)) { > + if (!descriptor->coll) { > + descriptor->coll = av_content_light_metadata_alloc(&descriptor->coll_size); > + if (!descriptor->coll) > + return AVERROR(ENOMEM); > + } Duplicated allocation here as well. See my comment on PATCH 1/3. /Tomas
> On 7 Sep 2020, at 10:46, Tomas Härdin <tjoppen@acc.umu.se> wrote: > > mån 2020-08-31 klockan 20:07 +0100 skrev Harry Mallon: >> * As embedded by Apple Compressor > > This needs a sample since it isn't part of any official spec, so that > we can have a test for this. I am attaching 3 files from Apple Compressor. They are all HDR10 (BT2020/PQ/metadata) files. I have tried to make them as small as possible but Compressor prefers them to be >=720 width for MXFs. They were all generated from the Netflix open content film Meridian, under creative commons. Meridian-Apple_ProResProxy-HDR10.mxf - MXF ProRes 422 Proxy, SMPTE mastering display metadata, Apple Content Light Level data Meridian-Apple_ProResProxy-HDR10.mov - Same as above but in a MOV wrapper, HDR10 metadata in MOV boxes Meridian-Apple-HEVC-HDR10.mov - Same sequence but HEVC 10bit in a MOV wrapper There are currently no fate tests that read HDR metadata so I figured these three would be a reasonable start. I had to make the Content Light Level data up as I do not have access to a program to calculate it (it's possible X265 can do it but I couldn't work it out). > >> + if (IS_KLV_KEY(uid, mxf_coll_apple_max_cll)) { >> + if (!descriptor->coll) { >> + descriptor->coll = av_content_light_metadata_alloc(&descriptor->coll_size); >> + if (!descriptor->coll) >> + return AVERROR(ENOMEM); >> + } > > Duplicated allocation here as well. See my comment on PATCH 1/3. > I will approach the rest of the comments in a separate email. Best, Harry
> On 7 Sep 2020, at 10:46, Tomas Härdin <tjoppen@acc.umu.se> wrote: > > mån 2020-08-31 klockan 20:07 +0100 skrev Harry Mallon: >> * As embedded by Apple Compressor > > This needs a sample since it isn't part of any official spec, so that > we can have a test for this. > >> + if (IS_KLV_KEY(uid, mxf_coll_apple_max_cll)) { >> + if (!descriptor->coll) { >> + descriptor->coll = av_content_light_metadata_alloc(&descriptor->coll_size); >> + if (!descriptor->coll) >> + return AVERROR(ENOMEM); >> + } > > Duplicated allocation here as well. See my comment on PATCH 1/3. > Sample and FATE test in v2. Best, Harry
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c index a7a1e74a0a..58a11384b4 100644 --- a/libavformat/mxfdec.c +++ b/libavformat/mxfdec.c @@ -216,6 +216,8 @@ typedef struct MXFDescriptor { UID color_trc_ul; UID color_space_ul; AVMasteringDisplayMetadata *mastering; + AVContentLightMetadata *coll; + size_t coll_size; } MXFDescriptor; typedef struct MXFIndexTableSegment { @@ -335,6 +337,8 @@ static const uint8_t mxf_mastering_display_primaries[] = { 0x06,0 static const uint8_t mxf_mastering_display_white_point_chromaticity[] = { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x20,0x04,0x01,0x01,0x02,0x00,0x00 }; static const uint8_t mxf_mastering_display_maximum_luminance[] = { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x20,0x04,0x01,0x01,0x03,0x00,0x00 }; static const uint8_t mxf_mastering_display_minimum_luminance[] = { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x20,0x04,0x01,0x01,0x04,0x00,0x00 }; +static const uint8_t mxf_coll_apple_max_cll[] = { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x0e,0x20,0x04,0x01,0x05,0x03,0x01,0x01 }; +static const uint8_t mxf_coll_apple_max_fall[] = { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x0e,0x20,0x04,0x01,0x05,0x03,0x01,0x02 }; #define IS_KLV_KEY(x, y) (!memcmp(x, y, sizeof(y))) @@ -345,6 +349,7 @@ static void mxf_free_metadataset(MXFMetadataSet **ctx, int freectx) case Descriptor: av_freep(&((MXFDescriptor *)*ctx)->extradata); av_freep(&((MXFDescriptor *)*ctx)->mastering); + av_freep(&((MXFDescriptor *)*ctx)->coll); break; case MultipleDescriptor: av_freep(&((MXFDescriptor *)*ctx)->sub_descriptors_refs); @@ -1334,6 +1339,22 @@ static int mxf_read_generic_descriptor(void *arg, AVIOContext *pb, int tag, int if (descriptor->mastering->max_luminance.den != 0) descriptor->mastering->has_luminance = 1; } + if (IS_KLV_KEY(uid, mxf_coll_apple_max_cll)) { + if (!descriptor->coll) { + descriptor->coll = av_content_light_metadata_alloc(&descriptor->coll_size); + if (!descriptor->coll) + return AVERROR(ENOMEM); + } + descriptor->coll->MaxCLL = avio_rb16(pb); + } + if (IS_KLV_KEY(uid, mxf_coll_apple_max_fall)) { + if (!descriptor->coll) { + descriptor->coll = av_content_light_metadata_alloc(&descriptor->coll_size); + if (!descriptor->coll) + return AVERROR(ENOMEM); + } + descriptor->coll->MaxFALL = avio_rb16(pb); + } break; } return 0; @@ -2602,6 +2623,14 @@ static int mxf_parse_structural_metadata(MXFContext *mxf) goto fail_and_free; descriptor->mastering = NULL; } + if (descriptor->coll) { + ret = av_stream_add_side_data(st, AV_PKT_DATA_CONTENT_LIGHT_LEVEL, + (uint8_t *)descriptor->coll, + descriptor->coll_size); + if (ret < 0) + goto fail_and_free; + descriptor->coll = NULL; + } } else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { container_ul = mxf_get_codec_ul(mxf_sound_essence_container_uls, essence_container_ul); /* Only overwrite existing codec ID if it is unset or A-law, which is the default according to SMPTE RP 224. */
* As embedded by Apple Compressor Signed-off-by: Harry Mallon <harry.mallon@codex.online> --- libavformat/mxfdec.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)