diff mbox

[FFmpeg-devel,v2,2/3] avformat/mxfenc: write reel_name if metadata key is present

Message ID 20171129041105.16748-3-mindmark@gmail.com
State Superseded
Headers show

Commit Message

Mark Reid Nov. 29, 2017, 4:11 a.m. UTC 
---
 libavformat/mxf.h    |  1 +
 libavformat/mxfenc.c | 42 +++++++++++++++++++++++++++++++++++-------
 2 files changed, 36 insertions(+), 7 deletions(-)

Comments

Tomas Härdin Nov. 29, 2017, 9:38 a.m. UTC | #1 
On 2017-11-29 05:11, Mark Reid wrote:
> ---
>   libavformat/mxf.h    |  1 +
>   libavformat/mxfenc.c | 42 +++++++++++++++++++++++++++++++++++-------
>   2 files changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/libavformat/mxf.h b/libavformat/mxf.h
> index 2d5b44943b..ffcc429a8b 100644
> --- a/libavformat/mxf.h
> +++ b/libavformat/mxf.h
> @@ -47,6 +47,7 @@ enum MXFMetadataSetType {
>       EssenceContainerData,
>       EssenceGroup,
>       TaggedValue,
> +    TapeDescriptor,
>   };
>   
>   enum MXFFrameLayout {
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index baaeb8c617..02192aa22b 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -105,6 +105,7 @@ typedef struct MXFPackage {
>       char *name;
>       enum MXFMetadataSetType type;
>       int instance;
> +    struct MXFPackage *ref;
>   } MXFPackage;
>   
>   enum ULIndex {
> @@ -991,20 +992,33 @@ static void mxf_write_structural_component(AVFormatContext *s, AVStream *st, MXF
>   
>       // write source package uid, end of the reference
>       mxf_write_local_tag(pb, 32, 0x1101);
> -    if (package->type == SourcePackage) {
> +    if (!package->ref) {
>           for (i = 0; i < 4; i++)
>               avio_wb64(pb, 0);
>       } else
> -        mxf_write_umid(s, 1);
> +        mxf_write_umid(s, package->ref->instance);
>   
>       // write source track id
>       mxf_write_local_tag(pb, 4, 0x1102);
> -    if (package->type == SourcePackage)
> +    if (package->type == SourcePackage && !package->ref)
>           avio_wb32(pb, 0);
>       else
>           avio_wb32(pb, st->index+2);
>   }
>   
> +static void mxf_write_tape_descriptor(AVFormatContext *s)
> +{
> +    AVIOContext *pb = s->pb;
> +
> +    mxf_write_metadata_key(pb, 0x012e00);
> +    PRINT_KEY(s, "tape descriptor key", pb->buf_ptr - 16);
> +    klv_encode_ber_length(pb, 20);
> +    mxf_write_local_tag(pb, 16, 0x3C0A);
> +    mxf_write_uuid(pb, TapeDescriptor, 0);
> +    PRINT_KEY(s, "tape_desc uid", pb->buf_ptr - 16);
> +}
> +
> +
>   static void mxf_write_multi_descriptor(AVFormatContext *s)
>   {
>       MXFContext *mxf = s->priv_data;
> @@ -1388,13 +1402,17 @@ static void mxf_write_package(AVFormatContext *s, MXFPackage *package)
>       }
>   
>       // write multiple descriptor reference
> -    if (package->type == SourcePackage) {
> +    if (package->type == SourcePackage && package->instance == 1) {
>           mxf_write_local_tag(pb, 16, 0x4701);
>           if (s->nb_streams > 1) {
>               mxf_write_uuid(pb, MultipleDescriptor, 0);
>               mxf_write_multi_descriptor(s);
>           } else
>               mxf_write_uuid(pb, SubDescriptor, 0);
> +    } else if (package->type == SourcePackage && package->instance == 2) {
> +        mxf_write_local_tag(pb, 16, 0x4701);
> +        mxf_write_uuid(pb, TapeDescriptor, 0);
> +        mxf_write_tape_descriptor(s);
>       }
>   
>       // write timecode track
> @@ -1410,7 +1428,7 @@ static void mxf_write_package(AVFormatContext *s, MXFPackage *package)
>           mxf_write_structural_component(s, st, package);
>           mxf->track_uuid_offset++;
>   
> -        if (package->type == SourcePackage) {
> +        if (package->type == SourcePackage && package->instance == 1) {
>               MXFStreamContext *sc = st->priv_data;
>               mxf_essence_container_uls[sc->index].write_desc(s, st);
>           }
> @@ -1445,12 +1463,13 @@ static int mxf_write_header_metadata_sets(AVFormatContext *s)
>       AVDictionaryEntry *entry = NULL;
>       AVStream *st = NULL;
>       int i;
> -
> -    MXFPackage packages[2] = {};
> +    MXFPackage packages[3] = {};
>       int package_count = 2;
>       packages[0].type = MaterialPackage;
>       packages[1].type = SourcePackage;
>       packages[1].instance = 1;
> +    packages[0].ref = &packages[1];
> +
>   
>       if (entry = av_dict_get(s->metadata, "material_package_name", NULL, 0))
>          packages[0].name = entry->value;
> @@ -1468,6 +1487,15 @@ static int mxf_write_header_metadata_sets(AVFormatContext *s)
>           }
>       }
>   
> +    entry = av_dict_get(s->metadata, "reel_name", NULL, 0);
> +    if (entry) {
> +        packages[2].name = entry->value;
> +        packages[2].type = SourcePackage;
> +        packages[2].instance = 2;
> +        packages[1].ref = &packages[2];
> +        package_count = 3;
> +    }
> +
>       mxf_write_preface(s);
>       mxf_write_identification(s);
>       mxf_write_content_storage(s, packages, package_count);

Looks OK

/Tomas
Matthias Troffaes Nov. 29, 2017, 8:56 p.m. UTC | #2 
Dear Mark,

On Wed, Nov 29, 2017 at 4:11 AM, Mark Reid <mindmark@gmail.com> wrote:
> @@ -1445,12 +1463,13 @@ static int mxf_write_header_metadata_sets(AVFormatContext *s)
>      AVDictionaryEntry *entry = NULL;
>      AVStream *st = NULL;
>      int i;
> -
> -    MXFPackage packages[2] = {};
> +    MXFPackage packages[3] = {};

Here, may I suggest

MXFPackage packages[3] = {0};

for C99 compliance? For instance, msvc does not support an empty
struct initializer. See for instance
https://stackoverflow.com/questions/17589533/is-an-empty-initializer-list-valid-c-code

Kind regards,
Matthias
Mark Reid Nov. 30, 2017, 4:19 a.m. UTC | #3 
On Wed, Nov 29, 2017 at 12:56 PM, Matthias Troffaes <
matthias.troffaes@gmail.com> wrote:

> Dear Mark,
>
> On Wed, Nov 29, 2017 at 4:11 AM, Mark Reid <mindmark@gmail.com> wrote:
> > @@ -1445,12 +1463,13 @@ static int mxf_write_header_metadata_sets(AVFormatContext
> *s)
> >      AVDictionaryEntry *entry = NULL;
> >      AVStream *st = NULL;
> >      int i;
> > -
> > -    MXFPackage packages[2] = {};
> > +    MXFPackage packages[3] = {};
>
> Here, may I suggest
>
> MXFPackage packages[3] = {0};
>
> for C99 compliance? For instance, msvc does not support an empty
> struct initializer. See for instance
> https://stackoverflow.com/questions/17589533/is-an-
> empty-initializer-list-valid-c-code
>
> Kind regards,
> Matthias
>

sorry about that! yes I'll send a patch fixing that.


> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Mark Reid Nov. 30, 2017, 4:53 a.m. UTC | #4 
On Wed, Nov 29, 2017 at 8:19 PM, Mark Reid <mindmark@gmail.com> wrote:

>
>
> On Wed, Nov 29, 2017 at 12:56 PM, Matthias Troffaes <
> matthias.troffaes@gmail.com> wrote:
>
>> Dear Mark,
>>
>> On Wed, Nov 29, 2017 at 4:11 AM, Mark Reid <mindmark@gmail.com> wrote:
>> > @@ -1445,12 +1463,13 @@ static int mxf_write_header_metadata_sets(AVFormatContext
>> *s)
>> >      AVDictionaryEntry *entry = NULL;
>> >      AVStream *st = NULL;
>> >      int i;
>> > -
>> > -    MXFPackage packages[2] = {};
>> > +    MXFPackage packages[3] = {};
>>
>> Here, may I suggest
>>
>> MXFPackage packages[3] = {0};
>>
>> for C99 compliance? For instance, msvc does not support an empty
>> struct initializer. See for instance
>> https://stackoverflow.com/questions/17589533/is-an-empty-
>> initializer-list-valid-c-code
>>
>> Kind regards,
>> Matthias
>>
>
> sorry about that! yes I'll send a patch fixing that.
>

clang give me a warning telling to do this instead
packages[3] = {{0}};
I assume thats correct as I see thats used throughout the codebase.


>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
>
Matthias Troffaes Nov. 30, 2017, 10:24 a.m. UTC | #5 
Dear Mark,

On Thu, Nov 30, 2017 at 4:53 AM, Mark Reid <mindmark@gmail.com> wrote:
> clang give me a warning telling to do this instead
> packages[3] = {{0}};
> I assume thats correct as I see thats used throughout the codebase.

Ah yes, that's perfect - good catch.

Kind regards,
Matthias
diff mbox

Patch

diff --git a/libavformat/mxf.h b/libavformat/mxf.h
index 2d5b44943b..ffcc429a8b 100644
--- a/libavformat/mxf.h
+++ b/libavformat/mxf.h
@@ -47,6 +47,7 @@  enum MXFMetadataSetType {
     EssenceContainerData,
     EssenceGroup,
     TaggedValue,
+    TapeDescriptor,
 };
 
 enum MXFFrameLayout {
diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index baaeb8c617..02192aa22b 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -105,6 +105,7 @@  typedef struct MXFPackage {
     char *name;
     enum MXFMetadataSetType type;
     int instance;
+    struct MXFPackage *ref;
 } MXFPackage;
 
 enum ULIndex {
@@ -991,20 +992,33 @@  static void mxf_write_structural_component(AVFormatContext *s, AVStream *st, MXF
 
     // write source package uid, end of the reference
     mxf_write_local_tag(pb, 32, 0x1101);
-    if (package->type == SourcePackage) {
+    if (!package->ref) {
         for (i = 0; i < 4; i++)
             avio_wb64(pb, 0);
     } else
-        mxf_write_umid(s, 1);
+        mxf_write_umid(s, package->ref->instance);
 
     // write source track id
     mxf_write_local_tag(pb, 4, 0x1102);
-    if (package->type == SourcePackage)
+    if (package->type == SourcePackage && !package->ref)
         avio_wb32(pb, 0);
     else
         avio_wb32(pb, st->index+2);
 }
 
+static void mxf_write_tape_descriptor(AVFormatContext *s)
+{
+    AVIOContext *pb = s->pb;
+
+    mxf_write_metadata_key(pb, 0x012e00);
+    PRINT_KEY(s, "tape descriptor key", pb->buf_ptr - 16);
+    klv_encode_ber_length(pb, 20);
+    mxf_write_local_tag(pb, 16, 0x3C0A);
+    mxf_write_uuid(pb, TapeDescriptor, 0);
+    PRINT_KEY(s, "tape_desc uid", pb->buf_ptr - 16);
+}
+
+
 static void mxf_write_multi_descriptor(AVFormatContext *s)
 {
     MXFContext *mxf = s->priv_data;
@@ -1388,13 +1402,17 @@  static void mxf_write_package(AVFormatContext *s, MXFPackage *package)
     }
 
     // write multiple descriptor reference
-    if (package->type == SourcePackage) {
+    if (package->type == SourcePackage && package->instance == 1) {
         mxf_write_local_tag(pb, 16, 0x4701);
         if (s->nb_streams > 1) {
             mxf_write_uuid(pb, MultipleDescriptor, 0);
             mxf_write_multi_descriptor(s);
         } else
             mxf_write_uuid(pb, SubDescriptor, 0);
+    } else if (package->type == SourcePackage && package->instance == 2) {
+        mxf_write_local_tag(pb, 16, 0x4701);
+        mxf_write_uuid(pb, TapeDescriptor, 0);
+        mxf_write_tape_descriptor(s);
     }
 
     // write timecode track
@@ -1410,7 +1428,7 @@  static void mxf_write_package(AVFormatContext *s, MXFPackage *package)
         mxf_write_structural_component(s, st, package);
         mxf->track_uuid_offset++;
 
-        if (package->type == SourcePackage) {
+        if (package->type == SourcePackage && package->instance == 1) {
             MXFStreamContext *sc = st->priv_data;
             mxf_essence_container_uls[sc->index].write_desc(s, st);
         }
@@ -1445,12 +1463,13 @@  static int mxf_write_header_metadata_sets(AVFormatContext *s)
     AVDictionaryEntry *entry = NULL;
     AVStream *st = NULL;
     int i;
-
-    MXFPackage packages[2] = {};
+    MXFPackage packages[3] = {};
     int package_count = 2;
     packages[0].type = MaterialPackage;
     packages[1].type = SourcePackage;
     packages[1].instance = 1;
+    packages[0].ref = &packages[1];
+
 
     if (entry = av_dict_get(s->metadata, "material_package_name", NULL, 0))
        packages[0].name = entry->value;
@@ -1468,6 +1487,15 @@  static int mxf_write_header_metadata_sets(AVFormatContext *s)
         }
     }
 
+    entry = av_dict_get(s->metadata, "reel_name", NULL, 0);
+    if (entry) {
+        packages[2].name = entry->value;
+        packages[2].type = SourcePackage;
+        packages[2].instance = 2;
+        packages[1].ref = &packages[2];
+        package_count = 3;
+    }
+
     mxf_write_preface(s);
     mxf_write_identification(s);
     mxf_write_content_storage(s, packages, package_count);