[FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support

[FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/mxf.c    |  3 +++
 libavformat/mxf.h    |  1 +
 libavformat/mxfdec.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+)

diff --git a/libavformat/mxf.c b/libavformat/mxf.c
index 36d662b58c..8ef928b8fc 100644
--- a/libavformat/mxf.c
+++ b/libavformat/mxf.c
@@ -66,6 +66,9 @@ const MXFCodecUL ff_mxf_codec_uls[] = {
     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x01,0x01,0x02,0x02,0x01 }, 16,       AV_CODEC_ID_V210 }, /* V210 */
     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02,0x11,0x00,0x00 }, 14,     AV_CODEC_ID_PRORES }, /* Avid MC7 ProRes */
     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x06,0x00,0x00 }, 14,     AV_CODEC_ID_PRORES }, /* Apple ProRes */
+    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09,0x01,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V0 */
+    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09,0x02,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V1 */
+    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09,0x04,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V3 */
     /* SoundEssenceCompression */
     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x03,0x04,0x02,0x02,0x02,0x03,0x03,0x01,0x00 }, 14,        AV_CODEC_ID_AAC }, /* MPEG-2 AAC ADTS (legacy) */
     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x01,0x00,0x00,0x00,0x00 }, 13,  AV_CODEC_ID_PCM_S16LE }, /* uncompressed */
diff --git a/libavformat/mxf.h b/libavformat/mxf.h
index 4d9f5119a3..2561605ce5 100644
--- a/libavformat/mxf.h
+++ b/libavformat/mxf.h
@@ -54,6 +54,7 @@ enum MXFMetadataSetType {
     AudioChannelLabelSubDescriptor,
     SoundfieldGroupLabelSubDescriptor,
     GroupOfSoundfieldGroupsLabelSubDescriptor,
+    FFV1SubDescriptor,
 };
 
 enum MXFFrameLayout {
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 400941c348..c4d9d6ed93 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -237,6 +237,12 @@ typedef struct MXFMCASubDescriptor {
     char *language;
 } MXFMCASubDescriptor;
 
+typedef struct MXFFFV1SubDescriptor {
+    MXFMetadataSet meta;
+    uint8_t *extradata;
+    int extradata_size;
+} MXFFFV1SubDescriptor;
+
 typedef struct MXFIndexTableSegment {
     MXFMetadataSet meta;
     int edit_unit_byte_count;
@@ -337,6 +343,7 @@ static const uint8_t mxf_crypto_source_container_ul[]      = { 0x06,0x0e,0x2b,0x
 static const uint8_t mxf_encrypted_triplet_key[]           = { 0x06,0x0e,0x2b,0x34,0x02,0x04,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x7e,0x01,0x00 };
 static const uint8_t mxf_encrypted_essence_container[]     = { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0b,0x01,0x00 };
 static const uint8_t mxf_sony_mpeg4_extradata[]            = { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0e,0x06,0x06,0x02,0x02,0x01,0x00,0x00 };
+static const uint8_t mxf_ffv1_extradata[]                  = { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x01,0x06,0x0c,0x01,0x00,0x00,0x00 };
 static const uint8_t mxf_avid_project_name[]               = { 0xa5,0xfb,0x7b,0x25,0xf6,0x15,0x94,0xb9,0x62,0xfc,0x37,0x17,0x49,0x2d,0x42,0xbf };
 static const uint8_t mxf_jp2k_rsiz[]                       = { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x01,0x00,0x00,0x00 };
 static const uint8_t mxf_indirect_value_utf16le[]          = { 0x4c,0x00,0x02,0x10,0x01,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01 };
@@ -377,6 +384,9 @@ static void mxf_free_metadataset(MXFMetadataSet **ctx, int freectx)
         av_freep(&((MXFDescriptor *)*ctx)->file_descriptors_refs);
         av_freep(&((MXFDescriptor *)*ctx)->sub_descriptors_refs);
         break;
+    case FFV1SubDescriptor:
+        av_freep(&((MXFFFV1SubDescriptor *)*ctx)->extradata);
+        break;
     case AudioChannelLabelSubDescriptor:
     case SoundfieldGroupLabelSubDescriptor:
     case GroupOfSoundfieldGroupsLabelSubDescriptor:
@@ -1473,6 +1483,25 @@ static int mxf_read_mca_sub_descriptor(void *arg, AVIOContext *pb, int tag, int
     return 0;
 }
 
+static int mxf_read_ffv1_sub_descriptor(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset)
+{
+    MXFFFV1SubDescriptor *ffv1_sub_descriptor = arg;
+
+    if (IS_KLV_KEY(uid, mxf_ffv1_extradata)) {
+        if (ffv1_sub_descriptor->extradata)
+            av_log(NULL, AV_LOG_WARNING, "Duplicate ffv1_extradata\n");
+        av_free(ffv1_sub_descriptor->extradata);
+        ffv1_sub_descriptor->extradata_size = 0;
+        ffv1_sub_descriptor->extradata = av_malloc(size);
+        if (!ffv1_sub_descriptor->extradata)
+            return AVERROR(ENOMEM);
+        ffv1_sub_descriptor->extradata_size = size;
+        avio_read(pb, ffv1_sub_descriptor->extradata, size);
+    }
+
+    return 0;
+}
+
 static int mxf_read_indirect_value(void *arg, AVIOContext *pb, int size)
 {
     MXFTaggedValue *tagged_value = arg;
@@ -1554,6 +1583,7 @@ static const MXFCodecUL mxf_picture_essence_container_uls[] = {
     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x1c,0x01,0x00 }, 14,     AV_CODEC_ID_PRORES, NULL, 14 }, /* ProRes */
     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x04,0x60,0x01 }, 14, AV_CODEC_ID_MPEG2VIDEO, NULL, 15 }, /* MPEG-ES */
     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x01,0x04,0x01 }, 14, AV_CODEC_ID_MPEG2VIDEO, NULL, 15, D10D11Wrap }, /* SMPTE D-10 mapping */
+    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0d,0x0d,0x01,0x03,0x01,0x02,0x23,0x01,0x00 }, 14,       AV_CODEC_ID_FFV1, NULL, 14 },
     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x02,0x41,0x01 }, 14,    AV_CODEC_ID_DVVIDEO, NULL, 15 }, /* DV 625 25mbps */
     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x05,0x00,0x00 }, 14,   AV_CODEC_ID_RAWVIDEO, NULL, 15, RawVWrap }, /* uncompressed picture */
     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0a,0x0e,0x0f,0x03,0x01,0x02,0x20,0x01,0x01 }, 15,     AV_CODEC_ID_HQ_HQA },
@@ -2444,6 +2474,21 @@ static MXFMCASubDescriptor *find_mca_link_id(MXFContext *mxf, enum MXFMetadataSe
     return NULL;
 }
 
+static void parse_ffv1_sub_descriptor(MXFContext *mxf, MXFTrack *source_track, MXFDescriptor *descriptor, AVStream *st)
+{
+    for (int i = 0; i < descriptor->sub_descriptors_count; i++) {
+        MXFFFV1SubDescriptor *ffv1_sub_descriptor = mxf_resolve_strong_ref(mxf, &descriptor->sub_descriptors_refs[i], FFV1SubDescriptor);
+        if (ffv1_sub_descriptor == NULL)
+            continue;
+
+        descriptor->extradata      = ffv1_sub_descriptor->extradata;
+        descriptor->extradata_size = ffv1_sub_descriptor->extradata_size;
+        ffv1_sub_descriptor->extradata = NULL;
+        ffv1_sub_descriptor->extradata_size = 0;
+        break;
+    }
+}
+
 static int parse_mca_labels(MXFContext *mxf, MXFTrack *source_track, MXFDescriptor *descriptor, AVStream *st)
 {
     uint64_t routing[FF_SANE_NB_CHANNELS] = {0};
@@ -2972,6 +3017,8 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
                 st->codecpar->codec_id = AV_CODEC_ID_EIA_608;
             }
         }
+        if (!descriptor->extradata)
+            parse_ffv1_sub_descriptor(mxf, source_track, descriptor, st);
         if (descriptor->extradata) {
             if (!ff_alloc_extradata(st->codecpar, descriptor->extradata_size)) {
                 memcpy(st->codecpar->extradata, descriptor->extradata, descriptor->extradata_size);
@@ -3159,6 +3206,7 @@ static const MXFMetadataReadTableEntry mxf_metadata_read_table[] = {
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x6b,0x00 }, mxf_read_mca_sub_descriptor, sizeof(MXFMCASubDescriptor), AudioChannelLabelSubDescriptor },
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x6c,0x00 }, mxf_read_mca_sub_descriptor, sizeof(MXFMCASubDescriptor), SoundfieldGroupLabelSubDescriptor },
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x6d,0x00 }, mxf_read_mca_sub_descriptor, sizeof(MXFMCASubDescriptor), GroupOfSoundfieldGroupsLabelSubDescriptor },
+    { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x81,0x03 }, mxf_read_ffv1_sub_descriptor, sizeof(MXFFFV1SubDescriptor), FFV1SubDescriptor },
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3A,0x00 }, mxf_read_track, sizeof(MXFTrack), Track }, /* Static Track */
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3B,0x00 }, mxf_read_track, sizeof(MXFTrack), Track }, /* Generic Track */
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x14,0x00 }, mxf_read_timecode_component, sizeof(MXFTimecodeComponent), TimecodeComponent },
-- 
2.17.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/3] avformat/mpc8: Check and propagate more errors

[Michael Niedermayer]

Fixes: Timeout
Fixes: 48846/clusterfuzz-testcase-minimized-ffmpeg_dem_MPC8_fuzzer-5278532493770752

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/mpc8.c | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/libavformat/mpc8.c b/libavformat/mpc8.c
index 2822a08b55..95a1529c5d 100644
--- a/libavformat/mpc8.c
+++ b/libavformat/mpc8.c
@@ -135,37 +135,40 @@ static void mpc8_get_chunk_header(AVIOContext *pb, int *tag, int64_t *size)
         *size += pos;
 }
 
-static void mpc8_parse_seektable(AVFormatContext *s, int64_t off)
+static int mpc8_parse_seektable(AVFormatContext *s, int64_t off)
 {
     MPCContext *c = s->priv_data;
     int tag;
     int64_t size, pos, ppos[2];
     uint8_t *buf;
     int i, t, seekd, ret;
+    int64_t ret64;
     GetBitContext gb;
 
     if (s->nb_streams == 0) {
         av_log(s, AV_LOG_ERROR, "No stream added before parsing seek table\n");
-        return;
+        return AVERROR_INVALIDDATA;
     }
 
-    avio_seek(s->pb, off, SEEK_SET);
+    ret64 = avio_seek(s->pb, off, SEEK_SET);
+    if (ret64 < 0)
+        return AVERROR_INVALIDDATA;
     mpc8_get_chunk_header(s->pb, &tag, &size);
-    if(tag != TAG_SEEKTABLE){
+    if(tag != TAG_SEEKTABLE || avio_feof(s->pb)){
         av_log(s, AV_LOG_ERROR, "No seek table at given position\n");
-        return;
+        return AVERROR_INVALIDDATA;
     }
     if (size > INT_MAX/10 || size<=0) {
         av_log(s, AV_LOG_ERROR, "Bad seek table size\n");
-        return;
+        return AVERROR_INVALIDDATA;
     }
     if(!(buf = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE)))
-        return;
+        return AVERROR(ENOMEM);
     ret = avio_read(s->pb, buf, size);
     if (ret != size) {
         av_log(s, AV_LOG_ERROR, "seek table truncated\n");
         av_free(buf);
-        return;
+        return AVERROR_INVALIDDATA;
     }
     memset(buf+size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
 
@@ -174,14 +177,14 @@ static void mpc8_parse_seektable(AVFormatContext *s, int64_t off)
     if(size > UINT_MAX/4 || size > c->samples/1152){
         av_log(s, AV_LOG_ERROR, "Seek table is too big\n");
         av_free(buf);
-        return;
+        return AVERROR_INVALIDDATA;
     }
     seekd = get_bits(&gb, 4);
     for(i = 0; i < 2; i++){
         pos = gb_get_v(&gb);
         if (av_sat_add64(pos, c->header_pos) != pos + (uint64_t)c->header_pos) {
             av_free(buf);
-            return;
+            return AVERROR_INVALIDDATA;
         }
 
         pos += c->header_pos;
@@ -191,7 +194,7 @@ static void mpc8_parse_seektable(AVFormatContext *s, int64_t off)
     for(; i < size; i++){
         if (get_bits_left(&gb) < 13) {
             av_free(buf);
-            return;
+            return AVERROR_INVALIDDATA;
         }
         t = get_unary(&gb, 1, 33) << 12;
         t += get_bits(&gb, 12);
@@ -203,26 +206,31 @@ static void mpc8_parse_seektable(AVFormatContext *s, int64_t off)
         ppos[0] = pos;
     }
     av_free(buf);
+    return 0;
 }
 
-static void mpc8_handle_chunk(AVFormatContext *s, int tag, int64_t chunk_pos, int64_t size)
+static int mpc8_handle_chunk(AVFormatContext *s, int tag, int64_t chunk_pos, int64_t size)
 {
     AVIOContext *pb = s->pb;
     int64_t pos, off;
+    int ret;
 
     switch(tag){
     case TAG_SEEKTBLOFF:
         pos = avio_tell(pb);
         off = ffio_read_varlen(pb);
         if (pos > INT64_MAX - size || off < 0 || off > INT64_MAX - chunk_pos)
-            return;
+            return AVERROR_INVALIDDATA;
         pos += size;
-        mpc8_parse_seektable(s, chunk_pos + off);
+        ret = mpc8_parse_seektable(s, chunk_pos + off);
+        if (ret < 0)
+            return AVERROR_INVALIDDATA;
         avio_seek(pb, pos, SEEK_SET);
         break;
     default:
         avio_skip(pb, size);
     }
+    return 0;
 }
 
 static int mpc8_read_header(AVFormatContext *s)
@@ -249,7 +257,9 @@ static int mpc8_read_header(AVFormatContext *s)
         }
         if(tag == TAG_STREAMHDR)
             break;
-        mpc8_handle_chunk(s, tag, pos, size);
+        ret = mpc8_handle_chunk(s, tag, pos, size);
+        if (ret < 0)
+            return ret;
     }
     if(tag != TAG_STREAMHDR){
         av_log(s, AV_LOG_ERROR, "Stream header not found\n");
-- 
2.17.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/mpc8: Check and propagate more errors

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 722 bytes --]

On Mon, Jul 11, 2022 at 11:44:16PM +0200, Michael Niedermayer wrote:
> Fixes: Timeout
> Fixes: 48846/clusterfuzz-testcase-minimized-ffmpeg_dem_MPC8_fuzzer-5278532493770752
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/mpc8.c | 40 +++++++++++++++++++++++++---------------
>  1 file changed, 25 insertions(+), 15 deletions(-)

will apply

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/3] tools/target_dec_fuzzer: Adjust threshold for ANM

[Michael Niedermayer]

Fixes: Timeout
Fixes: 48923/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ANM_fuzzer-6391662321991680

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 tools/target_dec_fuzzer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
index f44cc6f857..3ba87b7f1c 100644
--- a/tools/target_dec_fuzzer.c
+++ b/tools/target_dec_fuzzer.c
@@ -209,6 +209,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
     maxsamples = maxsamples_per_frame * maxiteration;
     switch (c->p.id) {
     case AV_CODEC_ID_AGM:         maxpixels  /= 1024;  break;
+    case AV_CODEC_ID_ANM:         maxpixels  /= 1024;  break;
     case AV_CODEC_ID_ARBC:        maxpixels  /= 1024;  break;
     case AV_CODEC_ID_ARGO:        maxpixels  /= 1024;  break;
     case AV_CODEC_ID_BINKVIDEO:   maxpixels  /= 32;    break;
-- 
2.17.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/3] tools/target_dec_fuzzer: Adjust threshold for ANM

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 757 bytes --]

On Mon, Jul 11, 2022 at 11:44:17PM +0200, Michael Niedermayer wrote:
> Fixes: Timeout
> Fixes: 48923/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ANM_fuzzer-6391662321991680
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  tools/target_dec_fuzzer.c | 1 +
>  1 file changed, 1 insertion(+)

will apply

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support

[Dave Rice]

> On Jul 11, 2022, at 5:44 PM, Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
> libavformat/mxf.c    |  3 +++
> libavformat/mxf.h    |  1 +
> libavformat/mxfdec.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 52 insertions(+)
> 
> diff --git a/libavformat/mxf.c b/libavformat/mxf.c
> index 36d662b58c..8ef928b8fc 100644
> --- a/libavformat/mxf.c
> +++ b/libavformat/mxf.c
> @@ -66,6 +66,9 @@ const MXFCodecUL ff_mxf_codec_uls[] = {
>     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x01,0x01,0x02,0x02,0x01 }, 16,       AV_CODEC_ID_V210 }, /* V210 */
>     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02,0x11,0x00,0x00 }, 14,     AV_CODEC_ID_PRORES }, /* Avid MC7 ProRes */
>     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x06,0x00,0x00 }, 14,     AV_CODEC_ID_PRORES }, /* Apple ProRes */
> +    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09,0x01,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V0 */
> +    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09,0x02,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V1 */
> +    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09,0x04,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V3 */
>     /* SoundEssenceCompression */
>     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x03,0x04,0x02,0x02,0x02,0x03,0x03,0x01,0x00 }, 14,        AV_CODEC_ID_AAC }, /* MPEG-2 AAC ADTS (legacy) */
>     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x01,0x00,0x00,0x00,0x00 }, 13,  AV_CODEC_ID_PCM_S16LE }, /* uncompressed */
> diff --git a/libavformat/mxf.h b/libavformat/mxf.h
> index 4d9f5119a3..2561605ce5 100644
> --- a/libavformat/mxf.h
> +++ b/libavformat/mxf.h
> @@ -54,6 +54,7 @@ enum MXFMetadataSetType {
>     AudioChannelLabelSubDescriptor,
>     SoundfieldGroupLabelSubDescriptor,
>     GroupOfSoundfieldGroupsLabelSubDescriptor,
> +    FFV1SubDescriptor,
> };
> 
> enum MXFFrameLayout {
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index 400941c348..c4d9d6ed93 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -237,6 +237,12 @@ typedef struct MXFMCASubDescriptor {
>     char *language;
> } MXFMCASubDescriptor;
> 
> +typedef struct MXFFFV1SubDescriptor {
> +    MXFMetadataSet meta;
> +    uint8_t *extradata;
> +    int extradata_size;
> +} MXFFFV1SubDescriptor;
> +
> typedef struct MXFIndexTableSegment {
>     MXFMetadataSet meta;
>     int edit_unit_byte_count;
> @@ -337,6 +343,7 @@ static const uint8_t mxf_crypto_source_container_ul[]      = { 0x06,0x0e,0x2b,0x
> static const uint8_t mxf_encrypted_triplet_key[]           = { 0x06,0x0e,0x2b,0x34,0x02,0x04,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x7e,0x01,0x00 };
> static const uint8_t mxf_encrypted_essence_container[]     = { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0b,0x01,0x00 };
> static const uint8_t mxf_sony_mpeg4_extradata[]            = { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0e,0x06,0x06,0x02,0x02,0x01,0x00,0x00 };
> +static const uint8_t mxf_ffv1_extradata[]                  = { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x01,0x06,0x0c,0x01,0x00,0x00,0x00 };
> static const uint8_t mxf_avid_project_name[]               = { 0xa5,0xfb,0x7b,0x25,0xf6,0x15,0x94,0xb9,0x62,0xfc,0x37,0x17,0x49,0x2d,0x42,0xbf };
> static const uint8_t mxf_jp2k_rsiz[]                       = { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x01,0x00,0x00,0x00 };
> static const uint8_t mxf_indirect_value_utf16le[]          = { 0x4c,0x00,0x02,0x10,0x01,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01 };
> @@ -377,6 +384,9 @@ static void mxf_free_metadataset(MXFMetadataSet **ctx, int freectx)
>         av_freep(&((MXFDescriptor *)*ctx)->file_descriptors_refs);
>         av_freep(&((MXFDescriptor *)*ctx)->sub_descriptors_refs);
>         break;
> +    case FFV1SubDescriptor:
> +        av_freep(&((MXFFFV1SubDescriptor *)*ctx)->extradata);
> +        break;
>     case AudioChannelLabelSubDescriptor:
>     case SoundfieldGroupLabelSubDescriptor:
>     case GroupOfSoundfieldGroupsLabelSubDescriptor:
> @@ -1473,6 +1483,25 @@ static int mxf_read_mca_sub_descriptor(void *arg, AVIOContext *pb, int tag, int
>     return 0;
> }
> 
> +static int mxf_read_ffv1_sub_descriptor(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset)
> +{
> +    MXFFFV1SubDescriptor *ffv1_sub_descriptor = arg;
> +
> +    if (IS_KLV_KEY(uid, mxf_ffv1_extradata)) {
> +        if (ffv1_sub_descriptor->extradata)
> +            av_log(NULL, AV_LOG_WARNING, "Duplicate ffv1_extradata\n");
> +        av_free(ffv1_sub_descriptor->extradata);
> +        ffv1_sub_descriptor->extradata_size = 0;
> +        ffv1_sub_descriptor->extradata = av_malloc(size);
> +        if (!ffv1_sub_descriptor->extradata)
> +            return AVERROR(ENOMEM);
> +        ffv1_sub_descriptor->extradata_size = size;
> +        avio_read(pb, ffv1_sub_descriptor->extradata, size);
> +    }
> +
> +    return 0;
> +}
> +
> static int mxf_read_indirect_value(void *arg, AVIOContext *pb, int size)
> {
>     MXFTaggedValue *tagged_value = arg;
> @@ -1554,6 +1583,7 @@ static const MXFCodecUL mxf_picture_essence_container_uls[] = {
>     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x1c,0x01,0x00 }, 14,     AV_CODEC_ID_PRORES, NULL, 14 }, /* ProRes */
>     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x04,0x60,0x01 }, 14, AV_CODEC_ID_MPEG2VIDEO, NULL, 15 }, /* MPEG-ES */
>     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x01,0x04,0x01 }, 14, AV_CODEC_ID_MPEG2VIDEO, NULL, 15, D10D11Wrap }, /* SMPTE D-10 mapping */
> +    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0d,0x0d,0x01,0x03,0x01,0x02,0x23,0x01,0x00 }, 14,       AV_CODEC_ID_FFV1, NULL, 14 },
>     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x02,0x41,0x01 }, 14,    AV_CODEC_ID_DVVIDEO, NULL, 15 }, /* DV 625 25mbps */
>     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x05,0x00,0x00 }, 14,   AV_CODEC_ID_RAWVIDEO, NULL, 15, RawVWrap }, /* uncompressed picture */
>     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0a,0x0e,0x0f,0x03,0x01,0x02,0x20,0x01,0x01 }, 15,     AV_CODEC_ID_HQ_HQA },
> @@ -2444,6 +2474,21 @@ static MXFMCASubDescriptor *find_mca_link_id(MXFContext *mxf, enum MXFMetadataSe
>     return NULL;
> }
> 
> +static void parse_ffv1_sub_descriptor(MXFContext *mxf, MXFTrack *source_track, MXFDescriptor *descriptor, AVStream *st)
> +{
> +    for (int i = 0; i < descriptor->sub_descriptors_count; i++) {
> +        MXFFFV1SubDescriptor *ffv1_sub_descriptor = mxf_resolve_strong_ref(mxf, &descriptor->sub_descriptors_refs[i], FFV1SubDescriptor);
> +        if (ffv1_sub_descriptor == NULL)
> +            continue;
> +
> +        descriptor->extradata      = ffv1_sub_descriptor->extradata;
> +        descriptor->extradata_size = ffv1_sub_descriptor->extradata_size;
> +        ffv1_sub_descriptor->extradata = NULL;
> +        ffv1_sub_descriptor->extradata_size = 0;
> +        break;
> +    }
> +}
> +
> static int parse_mca_labels(MXFContext *mxf, MXFTrack *source_track, MXFDescriptor *descriptor, AVStream *st)
> {
>     uint64_t routing[FF_SANE_NB_CHANNELS] = {0};
> @@ -2972,6 +3017,8 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
>                 st->codecpar->codec_id = AV_CODEC_ID_EIA_608;
>             }
>         }
> +        if (!descriptor->extradata)
> +            parse_ffv1_sub_descriptor(mxf, source_track, descriptor, st);
>         if (descriptor->extradata) {
>             if (!ff_alloc_extradata(st->codecpar, descriptor->extradata_size)) {
>                 memcpy(st->codecpar->extradata, descriptor->extradata, descriptor->extradata_size);
> @@ -3159,6 +3206,7 @@ static const MXFMetadataReadTableEntry mxf_metadata_read_table[] = {
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x6b,0x00 }, mxf_read_mca_sub_descriptor, sizeof(MXFMCASubDescriptor), AudioChannelLabelSubDescriptor },
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x6c,0x00 }, mxf_read_mca_sub_descriptor, sizeof(MXFMCASubDescriptor), SoundfieldGroupLabelSubDescriptor },
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x6d,0x00 }, mxf_read_mca_sub_descriptor, sizeof(MXFMCASubDescriptor), GroupOfSoundfieldGroupsLabelSubDescriptor },
> +    { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x81,0x03 }, mxf_read_ffv1_sub_descriptor, sizeof(MXFFFV1SubDescriptor), FFV1SubDescriptor },
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3A,0x00 }, mxf_read_track, sizeof(MXFTrack), Track }, /* Static Track */
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3B,0x00 }, mxf_read_track, sizeof(MXFTrack), Track }, /* Generic Track */
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x14,0x00 }, mxf_read_timecode_component, sizeof(MXFTimecodeComponent), TimecodeComponent },
> -- 
> 2.17.1

For those interested, this patch supports SMPTE RDD48 which is at https://www.digitizationguidelines.gov/guidelines/rdd48-2018_published.pdf <https://www.digitizationguidelines.gov/guidelines/rdd48-2018_published.pdf> and is one of SMPTE’s first documents published with a Creative Commons license. 
Dave Rice

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support

[Tomas Härdin]

mån 2022-07-11 klockan 23:44 +0200 skrev Michael Niedermayer:
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/mxf.c    |  3 +++
>  libavformat/mxf.h    |  1 +
>  libavformat/mxfdec.c | 48
> ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+)
> 
> diff --git a/libavformat/mxf.c b/libavformat/mxf.c
> index 36d662b58c..8ef928b8fc 100644
> --- a/libavformat/mxf.c
> +++ b/libavformat/mxf.c
> @@ -66,6 +66,9 @@ const MXFCodecUL ff_mxf_codec_uls[] = {
>      { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x01,0x01,0x02
> ,0x02,0x01 }, 16,       AV_CODEC_ID_V210 }, /* V210 */
>      { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02,0x11
> ,0x00,0x00 }, 14,     AV_CODEC_ID_PRORES }, /* Avid MC7 ProRes */
>      { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x06
> ,0x00,0x00 }, 14,     AV_CODEC_ID_PRORES }, /* Apple ProRes */
> +    { {
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09
> ,0x01,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V0 */
> +    { {
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09
> ,0x02,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V1 */
> +    { {
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09
> ,0x04,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V3 */

I do not see these ULs listed in the spec. Are they in some appropriate
RP?

> +static const uint8_t mxf_ffv1_extradata[]                  = {
> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x01,0x06,0x0c,0x01,0x00
> ,0x00,0x00 };

Nor do I see this UL. In fact the spec doesn't seem to mention FFV1 at
all unless my eyes deceive me.

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2235 bytes --]

On Mon, Jul 18, 2022 at 08:35:00PM +0200, Tomas Härdin wrote:
> mån 2022-07-11 klockan 23:44 +0200 skrev Michael Niedermayer:
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavformat/mxf.c    |  3 +++
> >  libavformat/mxf.h    |  1 +
> >  libavformat/mxfdec.c | 48
> > ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 52 insertions(+)
> > 
> > diff --git a/libavformat/mxf.c b/libavformat/mxf.c
> > index 36d662b58c..8ef928b8fc 100644
> > --- a/libavformat/mxf.c
> > +++ b/libavformat/mxf.c
> > @@ -66,6 +66,9 @@ const MXFCodecUL ff_mxf_codec_uls[] = {
> >      { {
> > 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x01,0x01,0x02
> > ,0x02,0x01 }, 16,       AV_CODEC_ID_V210 }, /* V210 */
> >      { {
> > 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02,0x11
> > ,0x00,0x00 }, 14,     AV_CODEC_ID_PRORES }, /* Avid MC7 ProRes */
> >      { {
> > 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x06
> > ,0x00,0x00 }, 14,     AV_CODEC_ID_PRORES }, /* Apple ProRes */
> > +    { {
> > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09
> > ,0x01,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V0 */
> > +    { {
> > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09
> > ,0x02,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V1 */
> > +    { {
> > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09
> > ,0x04,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V3 */
> 
> I do not see these ULs listed in the spec. Are they in some appropriate
> RP?
> 
> > +static const uint8_t mxf_ffv1_extradata[]                  = {
> > 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x01,0x06,0x0c,0x01,0x00
> > ,0x00,0x00 };
> 
> Nor do I see this UL. In fact the spec doesn't seem to mention FFV1 at
> all unless my eyes deceive me.

you need this:
SMPTE RDD 48:2018
Amendment 1:2022

i will update the patch to list this correctly

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"Nothing to hide" only works if the folks in power share the values of
you and everyone you know entirely and always will -- Tom Scott


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support

[Tomas Härdin]

mån 2022-07-11 klockan 23:44 +0200 skrev Michael Niedermayer:
> 
> +    { {
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09
> ,0x01,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V0 */
> +    { {
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09
> ,0x02,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V1 */
> +    { {

Double-checked, these are correct

> +typedef struct MXFFFV1SubDescriptor {
> +    MXFMetadataSet meta;
> +    uint8_t *extradata;
> +    int extradata_size;

Is FFV1 extradata size bounded? It so we could avoid an allocation.
Either way the local set syntax limits this to 64k, see below.

> +static const uint8_t mxf_ffv1_extradata[]                  = {
> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x01,0x06,0x0c,0x01,0x00
> ,0x00,0x00 };

Maybe add a comment // FFV1InitializationMetadata

> +static int mxf_read_ffv1_sub_descriptor(void *arg, AVIOContext *pb,
> int tag, int size, UID uid, int64_t klv_offset)
> +{
> +    MXFFFV1SubDescriptor *ffv1_sub_descriptor = arg;
> +
> +    if (IS_KLV_KEY(uid, mxf_ffv1_extradata)) {
> +        if (ffv1_sub_descriptor->extradata)
> +            av_log(NULL, AV_LOG_WARNING, "Duplicate
> ffv1_extradata\n");

Perhaps we should pass AVFormatContext* along with these functions to
enable proper logging. Doesn't need to hold up this patch though.

> +        av_free(ffv1_sub_descriptor->extradata);
> +        ffv1_sub_descriptor->extradata_size = 0;
> +        ffv1_sub_descriptor->extradata = av_malloc(size);
> +        if (!ffv1_sub_descriptor->extradata)
> +            return AVERROR(ENOMEM);
> +        ffv1_sub_descriptor->extradata_size = size;

This could be simplified with av_fast_malloc(), or no allocation at all
if we use a static sized array.

> +        avio_read(pb, ffv1_sub_descriptor->extradata, size);
> +    }

I presume the other items (GOP size, version number etc) are of no
interest and can be probed by the decoder.

> +    { {
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0d,0x0d,0x01,0x03,0x01,0x02,0x23
> ,0x01,0x00 }, 14,       AV_CODEC_ID_FFV1, NULL, 14 },

matching_len and wrapping_indicator_pos appear to be correct.
mxf_get_wrapping_kind() should report the correct wrapping type AFAICT.

> +static void parse_ffv1_sub_descriptor(MXFContext *mxf, MXFTrack
> *source_track, MXFDescriptor *descriptor, AVStream *st)
> +{
> +    for (int i = 0; i < descriptor->sub_descriptors_count; i++) {
> +        MXFFFV1SubDescriptor *ffv1_sub_descriptor =
> mxf_resolve_strong_ref(mxf, &descriptor->sub_descriptors_refs[i],
> FFV1SubDescriptor);
> +        if (ffv1_sub_descriptor == NULL)
> +            continue;
> +
> +        descriptor->extradata      = ffv1_sub_descriptor->extradata;
> +        descriptor->extradata_size = ffv1_sub_descriptor-
> >extradata_size;
> +        ffv1_sub_descriptor->extradata = NULL;
> +        ffv1_sub_descriptor->extradata_size = 0;

I guess this will require an allocation either way

> +    { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01
> ,0x81,0x03 }, mxf_read_ffv1_sub_descriptor,
> sizeof(MXFFFV1SubDescriptor), FFV1SubDescriptor },

The spec says 0x7F not 0x53. 0x53 is used in groups with 2-byte tags
rather than full KLVs. The intent here seems to be to use local tags,
which fortuitously limits extradata_size to 64k. This makes me think
Amendment 1:2022 is wrong or that 0x7F is just to signal private use
until it gets rolled into the next version of RDD 48.

Tables 18 and 23 in S377m-1-2009 say that 0x7F corresponds to "Abstract
Groups" which are "never encoded as Metadata Sets".

Reading S336m-2007 it seems one can actually use various lengths and
tag sizes. 0x53 corresponds to 2 byte length and 2 byte tag. S377m says
that in addition to this, 0x13 is allowed in MXF which uses ASN.1 BER
encoded lengths. Don't know if any files in the wild use that. Probably
not.

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 4953 bytes --]

On Tue, Jul 19, 2022 at 03:48:59PM +0200, Tomas Härdin wrote:
> mån 2022-07-11 klockan 23:44 +0200 skrev Michael Niedermayer:
> > 
> > +    { {
> > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09
> > ,0x01,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V0 */
> > +    { {
> > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09
> > ,0x02,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V1 */
> > +    { {
> 
> Double-checked, these are correct
> 
> > +typedef struct MXFFFV1SubDescriptor {
> > +    MXFMetadataSet meta;
> > +    uint8_t *extradata;
> > +    int extradata_size;
> 
> Is FFV1 extradata size bounded? It so we could avoid an allocation.
> Either way the local set syntax limits this to 64k, see below.

the extradata is extensible so future versions can be bigger.
For the current version there should be a maximum. As the extradata uses
an adaptive range coder it is not trivial to give a tight limit. It would
be easy to give some non tight limit. But iam not sure this has any point
as future versions can be bigger


> 
> > +static const uint8_t mxf_ffv1_extradata[]                  = {
> > 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x01,0x06,0x0c,0x01,0x00
> > ,0x00,0x00 };
> 
> Maybe add a comment // FFV1InitializationMetadata

done


> 
> > +static int mxf_read_ffv1_sub_descriptor(void *arg, AVIOContext *pb,
> > int tag, int size, UID uid, int64_t klv_offset)
> > +{
> > +    MXFFFV1SubDescriptor *ffv1_sub_descriptor = arg;
> > +
> > +    if (IS_KLV_KEY(uid, mxf_ffv1_extradata)) {
> > +        if (ffv1_sub_descriptor->extradata)
> > +            av_log(NULL, AV_LOG_WARNING, "Duplicate
> > ffv1_extradata\n");
> 
> Perhaps we should pass AVFormatContext* along with these functions to
> enable proper logging. Doesn't need to hold up this patch though.
> 
> > +        av_free(ffv1_sub_descriptor->extradata);
> > +        ffv1_sub_descriptor->extradata_size = 0;
> > +        ffv1_sub_descriptor->extradata = av_malloc(size);
> > +        if (!ffv1_sub_descriptor->extradata)
> > +            return AVERROR(ENOMEM);
> > +        ffv1_sub_descriptor->extradata_size = size;
> 
> This could be simplified with av_fast_malloc(), or no allocation at all
> if we use a static sized array.

this was missing AV_INPUT_BUFFER_PADDING_SIZE, added that.
I dont think av_fast_malloc() is a good idea, its a once per stream
allocation, i also dont think a static array is a good idea, there is
no size limit unless you want to limit to a specific version and
compute a worst case bound on a adaptive coder. And then
that worst case would be orders of magnitude bigger than real extradata
because real extradata compresses quite well. While the worst case would
be the case that is biggest and compresses worst. So a static array
would waste space


> 
> > +        avio_read(pb, ffv1_sub_descriptor->extradata, size);
> > +    }
> 
> I presume the other items (GOP size, version number etc) are of no
> interest and can be probed by the decoder.

They are of no interrest to me ATM. Maybe there is some use case for
parsing some mroe values, i do not really know. I would say we add them
when we understand what to do with them.
I presume theres at least one person who saw a use in them being stored
there so i presume theres someone in some standard committee who saw some
use in it. It seems that usecase did not make it into writing in the
part of the specification that i did read


[...]

> 
> > +    { {
> > 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01
> > ,0x81,0x03 }, mxf_read_ffv1_sub_descriptor,
> > sizeof(MXFFFV1SubDescriptor), FFV1SubDescriptor },
> 
> The spec says 0x7F not 0x53. 0x53 is used in groups with 2-byte tags

If i put 0x7F with no other change there, it will break demuxing the files i have
I guess i must have copied this from the files without noticing it mismatches
the spec


> rather than full KLVs. The intent here seems to be to use local tags,
> which fortuitously limits extradata_size to 64k. This makes me think
> Amendment 1:2022 is wrong or that 0x7F is just to signal private use
> until it gets rolled into the next version of RDD 48.
> 
> Tables 18 and 23 in S377m-1-2009 say that 0x7F corresponds to "Abstract
> Groups" which are "never encoded as Metadata Sets".
> 
> Reading S336m-2007 it seems one can actually use various lengths and
> tag sizes. 0x53 corresponds to 2 byte length and 2 byte tag. S377m says
> that in addition to this, 0x13 is allowed in MXF which uses ASN.1 BER
> encoded lengths. Don't know if any files in the wild use that. Probably
> not.

couldnt they make this more complex and bizarr?

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support

[Tomas Härdin]

fre 2022-07-29 klockan 01:18 +0200 skrev Michael Niedermayer:
> On Tue, Jul 19, 2022 at 03:48:59PM +0200, Tomas Härdin wrote:
> > mån 2022-07-11 klockan 23:44 +0200 skrev Michael Niedermayer:
> > > 
> > > +    { {
> > > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,
> > > 0x09
> > > ,0x01,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V0 */
> > > +    { {
> > > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,
> > > 0x09
> > > ,0x02,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V1 */
> > > +    { {
> > 
> > Double-checked, these are correct
> > 
> > > +typedef struct MXFFFV1SubDescriptor {
> > > +    MXFMetadataSet meta;
> > > +    uint8_t *extradata;
> > > +    int extradata_size;
> > 
> > Is FFV1 extradata size bounded? It so we could avoid an allocation.
> > Either way the local set syntax limits this to 64k, see below.
> 
> the extradata is extensible so future versions can be bigger.
> For the current version there should be a maximum. As the extradata
> uses
> an adaptive range coder it is not trivial to give a tight limit. It
> would
> be easy to give some non tight limit. But iam not sure this has any
> point
> as future versions can be bigger
> [...]
> i also dont think a static array is a good idea, there is
> no size limit unless you want to limit to a specific version and
> compute a worst case bound on a adaptive coder. And then
> that worst case would be orders of magnitude bigger than real
> extradata
> because real extradata compresses quite well. While the worst case
> would
> be the case that is biggest and compresses worst. So a static array
> would waste space

Values in (0x53) local sets are limited to 64k, so it should be fine in
this context


> > 
> > > +    { {
> > > 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,
> > > 0x01
> > > ,0x81,0x03 }, mxf_read_ffv1_sub_descriptor,
> > > sizeof(MXFFFV1SubDescriptor), FFV1SubDescriptor },
> > 
> > The spec says 0x7F not 0x53. 0x53 is used in groups with 2-byte
> > tags
> 
> If i put 0x7F with no other change there, it will break demuxing the
> files i have
> I guess i must have copied this from the files without noticing it
> mismatches
> the spec

Yeah I would expect it to break with 0x7F. Perhaps this will change
when the spec becomes official. If you have contact with the people
involved in this then I suggest asking them about this. It could also
be a typo in the spec.

> 
> 
> > rather than full KLVs. The intent here seems to be to use local
> > tags,
> > which fortuitously limits extradata_size to 64k. This makes me
> > think
> > Amendment 1:2022 is wrong or that 0x7F is just to signal private
> > use
> > until it gets rolled into the next version of RDD 48.
> > 
> > Tables 18 and 23 in S377m-1-2009 say that 0x7F corresponds to
> > "Abstract
> > Groups" which are "never encoded as Metadata Sets".
> > 
> > Reading S336m-2007 it seems one can actually use various lengths
> > and
> > tag sizes. 0x53 corresponds to 2 byte length and 2 byte tag. S377m
> > says
> > that in addition to this, 0x13 is allowed in MXF which uses ASN.1
> > BER
> > encoded lengths. Don't know if any files in the wild use that.
> > Probably
> > not.
> 
> couldnt they make this more complex and bizarr?

Welcome to the world of MXF.

The flip side here is that extradata over 64k *can* be encoded legally,
but I have never seen it in the wild and mxfdec doesn't support it
(yet). In that light it makes sense to keep the extradata allocation
dynamic

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support

[Pierre-Anthony Lemieux]

On Fri, Jul 29, 2022 at 6:15 AM Tomas Härdin <tjoppen@acc.umu.se> wrote:
>
> fre 2022-07-29 klockan 01:18 +0200 skrev Michael Niedermayer:
> > On Tue, Jul 19, 2022 at 03:48:59PM +0200, Tomas Härdin wrote:
> > > mån 2022-07-11 klockan 23:44 +0200 skrev Michael Niedermayer:
> > > >
> > > > +    { {
> > > > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,
> > > > 0x09
> > > > ,0x01,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V0 */
> > > > +    { {
> > > > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,
> > > > 0x09
> > > > ,0x02,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V1 */
> > > > +    { {
> > >
> > > Double-checked, these are correct

I recommend the draft SMPTE metadata registry at the following as the
reference for ULs:

https://registry.smpte-ra.org/apps/pages/

The registry is kept up-to-date, machine readable and free to access.

> > >
> > > > +typedef struct MXFFFV1SubDescriptor {
> > > > +    MXFMetadataSet meta;
> > > > +    uint8_t *extradata;
> > > > +    int extradata_size;
> > >
> > > Is FFV1 extradata size bounded? It so we could avoid an allocation.
> > > Either way the local set syntax limits this to 64k, see below.
> >
> > the extradata is extensible so future versions can be bigger.
> > For the current version there should be a maximum. As the extradata
> > uses
> > an adaptive range coder it is not trivial to give a tight limit. It
> > would
> > be easy to give some non tight limit. But iam not sure this has any
> > point
> > as future versions can be bigger
> > [...]
> > i also dont think a static array is a good idea, there is
> > no size limit unless you want to limit to a specific version and
> > compute a worst case bound on a adaptive coder. And then
> > that worst case would be orders of magnitude bigger than real
> > extradata
> > because real extradata compresses quite well. While the worst case
> > would
> > be the case that is biggest and compresses worst. So a static array
> > would waste space
>
> Values in (0x53) local sets are limited to 64k, so it should be fine in
> this context
>
>
> > >
> > > > +    { {
> > > > 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,
> > > > 0x01
> > > > ,0x81,0x03 }, mxf_read_ffv1_sub_descriptor,
> > > > sizeof(MXFFFV1SubDescriptor), FFV1SubDescriptor },
> > >
> > > The spec says 0x7F not 0x53. 0x53 is used in groups with 2-byte
> > > tags
> >
> > If i put 0x7F with no other change there, it will break demuxing the
> > files i have
> > I guess i must have copied this from the files without noticing it
> > mismatches
> > the spec
>
> Yeah I would expect it to break with 0x7F. Perhaps this will change
> when the spec becomes official. If you have contact with the people
> involved in this then I suggest asking them about this. It could also
> be a typo in the spec.

Byte 6 of Group ULs is set by convention to the wildcard value 0x7F to
indicate that the encoding of the Group is not limited to 0x53 (local
set with 2-byte local tags and length field). See the following:

https://registry.smpte-ra.org/view/draft/docs/Submissions%20Overview/Document-Editors-Information--Style-Guide/#groups-ul

MXF restricts header metadata to local sets with 2-byte local tags and
2-byte or BER lengths, so byte 6 can be 0x13 or 0x53.

>
> >
> >
> > > rather than full KLVs. The intent here seems to be to use local
> > > tags,
> > > which fortuitously limits extradata_size to 64k. This makes me
> > > think
> > > Amendment 1:2022 is wrong or that 0x7F is just to signal private
> > > use
> > > until it gets rolled into the next version of RDD 48.
> > >
> > > Tables 18 and 23 in S377m-1-2009 say that 0x7F corresponds to
> > > "Abstract
> > > Groups" which are "never encoded as Metadata Sets".
> > >
> > > Reading S336m-2007 it seems one can actually use various lengths
> > > and
> > > tag sizes. 0x53 corresponds to 2 byte length and 2 byte tag. S377m
> > > says
> > > that in addition to this, 0x13 is allowed in MXF which uses ASN.1
> > > BER
> > > encoded lengths. Don't know if any files in the wild use that.
> > > Probably
> > > not.
> >
> > couldnt they make this more complex and bizarr?
>
> Welcome to the world of MXF.
>
> The flip side here is that extradata over 64k *can* be encoded legally,
> but I have never seen it in the wild and mxfdec doesn't support it
> (yet). In that light it makes sense to keep the extradata allocation
> dynamic
>
> /Tomas
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support

[Tomas Härdin]

fre 2022-07-29 klockan 14:14 +0200 skrev Pierre-Anthony Lemieux:
> On Fri, Jul 29, 2022 at 6:15 AM Tomas Härdin <tjoppen@acc.umu.se>
> wrote:
> > 
> > fre 2022-07-29 klockan 01:18 +0200 skrev Michael Niedermayer:
> > > On Tue, Jul 19, 2022 at 03:48:59PM +0200, Tomas Härdin wrote:
> > > > mån 2022-07-11 klockan 23:44 +0200 skrev Michael Niedermayer:
> > > > > 
> > > > > +    { {
> > > > > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0
> > > > > x03,
> > > > > 0x09
> > > > > ,0x01,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V0 */
> > > > > +    { {
> > > > > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0
> > > > > x03,
> > > > > 0x09
> > > > > ,0x02,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V1 */
> > > > > +    { {
> > > > 
> > > > Double-checked, these are correct
> 
> I recommend the draft SMPTE metadata registry at the following as the
> reference for ULs:
> 
> https://registry.smpte-ra.org/apps/pages/
> 
> The registry is kept up-to-date, machine readable and free to access.

Neato. I actually have a tool for parsing ULs that I call wtful. For
now it parses relevant RP spreadsheets, but it's kinda shitty. Maybe I
can improve and publish it.
> 

> > > > 
> > > > > +    { {
> > > > > 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0
> > > > > x01,
> > > > > 0x01
> > > > > ,0x81,0x03 }, mxf_read_ffv1_sub_descriptor,
> > > > > sizeof(MXFFFV1SubDescriptor), FFV1SubDescriptor },
> > > > 
> > > > The spec says 0x7F not 0x53. 0x53 is used in groups with 2-byte
> > > > tags
> > > 
> > > If i put 0x7F with no other change there, it will break demuxing
> > > the
> > > files i have
> > > I guess i must have copied this from the files without noticing
> > > it
> > > mismatches
> > > the spec
> > 
> > Yeah I would expect it to break with 0x7F. Perhaps this will change
> > when the spec becomes official. If you have contact with the people
> > involved in this then I suggest asking them about this. It could
> > also
> > be a typo in the spec.
> 
> Byte 6 of Group ULs is set by convention to the wildcard value 0x7F
> to
> indicate that the encoding of the Group is not limited to 0x53 (local
> set with 2-byte local tags and length field). See the following:
> 
>   
> https://registry.smpte-ra.org/view/draft/docs/Submissions%20Overview/Document-Editors-Information--Style-Guide/#groups-ul
> 
> MXF restricts header metadata to local sets with 2-byte local tags
> and
> 2-byte or BER lengths, so byte 6 can be 0x13 or 0x53.

Alright, then it's fine. Maybe at some point we'll need to implement
BER lengths in local sets, but not now

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support

[Pierre-Anthony Lemieux]

On Fri, Jul 29, 2022 at 4:19 PM Tomas Härdin <tjoppen@acc.umu.se> wrote:
>
> fre 2022-07-29 klockan 14:14 +0200 skrev Pierre-Anthony Lemieux:
> > On Fri, Jul 29, 2022 at 6:15 AM Tomas Härdin <tjoppen@acc.umu.se>
> > wrote:
> > >
> > > fre 2022-07-29 klockan 01:18 +0200 skrev Michael Niedermayer:
> > > > On Tue, Jul 19, 2022 at 03:48:59PM +0200, Tomas Härdin wrote:
> > > > > mån 2022-07-11 klockan 23:44 +0200 skrev Michael Niedermayer:
> > > > > >
> > > > > > +    { {
> > > > > > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0
> > > > > > x03,
> > > > > > 0x09
> > > > > > ,0x01,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V0 */
> > > > > > +    { {
> > > > > > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0
> > > > > > x03,
> > > > > > 0x09
> > > > > > ,0x02,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V1 */
> > > > > > +    { {
> > > > >
> > > > > Double-checked, these are correct
> >
> > I recommend the draft SMPTE metadata registry at the following as the
> > reference for ULs:
> >
> > https://registry.smpte-ra.org/apps/pages/
> >
> > The registry is kept up-to-date, machine readable and free to access.
>
> Neato. I actually have a tool for parsing ULs that I call wtful. For
> now it parses relevant RP spreadsheets, but it's kinda shitty. Maybe I
> can improve and publish it.

The canonical format for the SMPTE registers is XML. The data (and
corresponding XSDs) can also be found at:

https://registry.smpte-ra.org/apps/pages/draft/

("draft" registries are 99% correct. "published" are 99.9% correct but
lag by about 6 months.)

The following are Java bindings for folks that are into that :)

https://github.com/sandflow/regxmllib/tree/master/src/main/java/com/sandflow/smpte/register

> >
>
> > > > >
> > > > > > +    { {
> > > > > > 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0
> > > > > > x01,
> > > > > > 0x01
> > > > > > ,0x81,0x03 }, mxf_read_ffv1_sub_descriptor,
> > > > > > sizeof(MXFFFV1SubDescriptor), FFV1SubDescriptor },
> > > > >
> > > > > The spec says 0x7F not 0x53. 0x53 is used in groups with 2-byte
> > > > > tags
> > > >
> > > > If i put 0x7F with no other change there, it will break demuxing
> > > > the
> > > > files i have
> > > > I guess i must have copied this from the files without noticing
> > > > it
> > > > mismatches
> > > > the spec
> > >
> > > Yeah I would expect it to break with 0x7F. Perhaps this will change
> > > when the spec becomes official. If you have contact with the people
> > > involved in this then I suggest asking them about this. It could
> > > also
> > > be a typo in the spec.
> >
> > Byte 6 of Group ULs is set by convention to the wildcard value 0x7F
> > to
> > indicate that the encoding of the Group is not limited to 0x53 (local
> > set with 2-byte local tags and length field). See the following:
> >
> >
> > https://registry.smpte-ra.org/view/draft/docs/Submissions%20Overview/Document-Editors-Information--Style-Guide/#groups-ul
> >
> > MXF restricts header metadata to local sets with 2-byte local tags
> > and
> > 2-byte or BER lengths, so byte 6 can be 0x13 or 0x53.
>
> Alright, then it's fine. Maybe at some point we'll need to implement
> BER lengths in local sets, but not now
>
> /Tomas
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".