diff mbox

[FFmpeg-devel,1/4] avformat/mxfenc: pass MXFPackage around instead of type

Message ID 20171127054219.37859-2-mindmark@gmail.com
State Accepted
Commit 62f7f40caa70212cd017a66d514fc64f24a3adbc
Headers show

Commit Message

Mark Reid Nov. 27, 2017, 5:42 a.m. UTC 
---
 libavformat/mxfenc.c | 99 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 55 insertions(+), 44 deletions(-)

Comments

Tomas Härdin Nov. 27, 2017, 10 a.m. UTC | #1 
On Sun, 2017-11-26 at 21:42 -0800, Mark Reid wrote:
> ---
>  libavformat/mxfenc.c | 99 +++++++++++++++++++++++++++++-------------
> ----------
>  1 file changed, 55 insertions(+), 44 deletions(-)
> 
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index 035e65ed43..ed6ecbf541 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -101,6 +101,12 @@ typedef struct MXFContainerEssenceEntry {
>      void (*write_desc)(AVFormatContext *, AVStream *);
>  } MXFContainerEssenceEntry;
>  
> +typedef struct MXFPackage {
> +    char *name;
> +    enum MXFMetadataSetType type;
> +    int instance;
> +} MXFPackage;
> [...]

Looks OK. This would make it easier if someone is crazy enough to
implement the higher op types in the future. I still maintain that
libmxf/bmxlib is better for that however

/Tomas
Michael Niedermayer Nov. 28, 2017, 8:09 p.m. UTC | #2 
On Mon, Nov 27, 2017 at 11:00:51AM +0100, Tomas Härdin wrote:
> On Sun, 2017-11-26 at 21:42 -0800, Mark Reid wrote:
> > ---
> >  libavformat/mxfenc.c | 99 +++++++++++++++++++++++++++++-------------
> > ----------
> >  1 file changed, 55 insertions(+), 44 deletions(-)
> > 
> > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> > index 035e65ed43..ed6ecbf541 100644
> > --- a/libavformat/mxfenc.c
> > +++ b/libavformat/mxfenc.c
> > @@ -101,6 +101,12 @@ typedef struct MXFContainerEssenceEntry {
> >      void (*write_desc)(AVFormatContext *, AVStream *);
> >  } MXFContainerEssenceEntry;
> >  
> > +typedef struct MXFPackage {
> > +    char *name;
> > +    enum MXFMetadataSetType type;
> > +    int instance;
> > +} MXFPackage;
> > [...]
> 
> Looks OK.

will apply

thanks


> This would make it easier if someone is crazy enough to
> implement the higher op types in the future. I still maintain that
> libmxf/bmxlib is better for that however
> 
> /Tomas



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Tomas Härdin Nov. 28, 2017, 11:05 p.m. UTC | #3 
On Tue, 2017-11-28 at 21:09 +0100, Michael Niedermayer wrote:
> On Mon, Nov 27, 2017 at 11:00:51AM +0100, Tomas Härdin wrote:
> > On Sun, 2017-11-26 at 21:42 -0800, Mark Reid wrote:
> > > ---
> > >  libavformat/mxfenc.c | 99 +++++++++++++++++++++++++++++---------
> > > ----
> > > ----------
> > >  1 file changed, 55 insertions(+), 44 deletions(-)
> > > 
> > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> > > index 035e65ed43..ed6ecbf541 100644
> > > --- a/libavformat/mxfenc.c
> > > +++ b/libavformat/mxfenc.c
> > > @@ -101,6 +101,12 @@ typedef struct MXFContainerEssenceEntry {
> > >      void (*write_desc)(AVFormatContext *, AVStream *);
> > >  } MXFContainerEssenceEntry;
> > >  
> > > +typedef struct MXFPackage {
> > > +    char *name;
> > > +    enum MXFMetadataSetType type;
> > > +    int instance;
> > > +} MXFPackage;
> > > [...]
> > 
> > Looks OK.
> 
> will apply
> 
> thanks

It sounded like he's working on an alternate patchset, see the PATCH
3/4 thread

/Tomas
Mark Reid Nov. 29, 2017, 2:04 a.m. UTC | #4 
On Tue, Nov 28, 2017 at 3:05 PM, Tomas Härdin <tjoppen@acc.umu.se> wrote:

> On Tue, 2017-11-28 at 21:09 +0100, Michael Niedermayer wrote:
> > On Mon, Nov 27, 2017 at 11:00:51AM +0100, Tomas Härdin wrote:
> > > On Sun, 2017-11-26 at 21:42 -0800, Mark Reid wrote:
> > > > ---
> > > >  libavformat/mxfenc.c | 99 +++++++++++++++++++++++++++++---------
> > > > ----
> > > > ----------
> > > >  1 file changed, 55 insertions(+), 44 deletions(-)
> > > >
> > > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> > > > index 035e65ed43..ed6ecbf541 100644
> > > > --- a/libavformat/mxfenc.c
> > > > +++ b/libavformat/mxfenc.c
> > > > @@ -101,6 +101,12 @@ typedef struct MXFContainerEssenceEntry {
> > > >      void (*write_desc)(AVFormatContext *, AVStream *);
> > > >  } MXFContainerEssenceEntry;
> > > >
> > > > +typedef struct MXFPackage {
> > > > +    char *name;
> > > > +    enum MXFMetadataSetType type;
> > > > +    int instance;
> > > > +} MXFPackage;
> > > > [...]
> > >
> > > Looks OK.
> >
> > will apply
> >
> > thanks
>
> It sounded like he's working on an alternate patchset, see the PATCH
> 3/4 thread
>

I see this was already applied, this one wasn't going to change in my new
patchset so it should be okay.


> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
diff mbox

Patch

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 035e65ed43..ed6ecbf541 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -101,6 +101,12 @@  typedef struct MXFContainerEssenceEntry {
     void (*write_desc)(AVFormatContext *, AVStream *);
 } MXFContainerEssenceEntry;
 
+typedef struct MXFPackage {
+    char *name;
+    enum MXFMetadataSetType type;
+    int instance;
+} MXFPackage;
+
 enum ULIndex {
     INDEX_MPEG2 = 0,
     INDEX_AES3,
@@ -808,13 +814,14 @@  static void mxf_write_identification(AVFormatContext *s)
     avio_wb64(pb, mxf->timestamp);
 }
 
-static void mxf_write_content_storage(AVFormatContext *s)
+static void mxf_write_content_storage(AVFormatContext *s, MXFPackage *packages, int package_count)
 {
     AVIOContext *pb = s->pb;
+    int i;
 
     mxf_write_metadata_key(pb, 0x011800);
     PRINT_KEY(s, "content storage key", pb->buf_ptr - 16);
-    klv_encode_ber_length(pb, 92);
+    klv_encode_ber_length(pb, 60 + (16 * package_count));
 
     // write uid
     mxf_write_local_tag(pb, 16, 0x3C0A);
@@ -822,10 +829,11 @@  static void mxf_write_content_storage(AVFormatContext *s)
     PRINT_KEY(s, "content storage uid", pb->buf_ptr - 16);
 
     // write package reference
-    mxf_write_local_tag(pb, 16 * 2 + 8, 0x1901);
-    mxf_write_refs_count(pb, 2);
-    mxf_write_uuid(pb, MaterialPackage, 0);
-    mxf_write_uuid(pb, SourcePackage, 0);
+    mxf_write_local_tag(pb, 16 * package_count + 8, 0x1901);
+    mxf_write_refs_count(pb, package_count);
+    for (i = 0; i < package_count; i++) {
+        mxf_write_uuid(pb, packages[i].type, packages[i].instance);
+    }
 
     // write essence container data
     mxf_write_local_tag(pb, 8 + 16, 0x1902);
@@ -833,7 +841,7 @@  static void mxf_write_content_storage(AVFormatContext *s)
     mxf_write_uuid(pb, EssenceContainerData, 0);
 }
 
-static void mxf_write_track(AVFormatContext *s, AVStream *st, enum MXFMetadataSetType type)
+static void mxf_write_track(AVFormatContext *s, AVStream *st, MXFPackage *package)
 {
     MXFContext *mxf = s->priv_data;
     AVIOContext *pb = s->pb;
@@ -845,7 +853,7 @@  static void mxf_write_track(AVFormatContext *s, AVStream *st, enum MXFMetadataSe
 
     // write track uid
     mxf_write_local_tag(pb, 16, 0x3C0A);
-    mxf_write_uuid(pb, type == MaterialPackage ? Track : Track + TypeBottom, st->index);
+    mxf_write_uuid(pb, package->type == MaterialPackage ? Track : Track + TypeBottom, st->index);
     PRINT_KEY(s, "track uid", pb->buf_ptr - 16);
 
     // write track id
@@ -854,7 +862,7 @@  static void mxf_write_track(AVFormatContext *s, AVStream *st, enum MXFMetadataSe
 
     // write track number
     mxf_write_local_tag(pb, 4, 0x4804);
-    if (type == MaterialPackage)
+    if (package->type == MaterialPackage)
         avio_wb32(pb, 0); // track number of material package is 0
     else
         avio_write(pb, sc->track_essence_element_key + 12, 4);
@@ -876,7 +884,7 @@  static void mxf_write_track(AVFormatContext *s, AVStream *st, enum MXFMetadataSe
 
     // write sequence refs
     mxf_write_local_tag(pb, 16, 0x4803);
-    mxf_write_uuid(pb, type == MaterialPackage ? Sequence: Sequence + TypeBottom, st->index);
+    mxf_write_uuid(pb, package->type == MaterialPackage ? Sequence: Sequence + TypeBottom, st->index);
 }
 
 static const uint8_t smpte_12m_timecode_track_data_ul[] = { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x01,0x03,0x02,0x01,0x01,0x00,0x00,0x00 };
@@ -905,7 +913,7 @@  static void mxf_write_common_fields(AVFormatContext *s, AVStream *st)
     }
 }
 
-static void mxf_write_sequence(AVFormatContext *s, AVStream *st, enum MXFMetadataSetType type)
+static void mxf_write_sequence(AVFormatContext *s, AVStream *st, MXFPackage *package)
 {
     MXFContext *mxf = s->priv_data;
     AVIOContext *pb = s->pb;
@@ -916,7 +924,7 @@  static void mxf_write_sequence(AVFormatContext *s, AVStream *st, enum MXFMetadat
     klv_encode_ber_length(pb, 80);
 
     mxf_write_local_tag(pb, 16, 0x3C0A);
-    mxf_write_uuid(pb, type == MaterialPackage ? Sequence: Sequence + TypeBottom, st->index);
+    mxf_write_uuid(pb, package->type == MaterialPackage ? Sequence: Sequence + TypeBottom, st->index);
 
     PRINT_KEY(s, "sequence uid", pb->buf_ptr - 16);
     mxf_write_common_fields(s, st);
@@ -928,12 +936,12 @@  static void mxf_write_sequence(AVFormatContext *s, AVStream *st, enum MXFMetadat
         component = TimecodeComponent;
     else
         component = SourceClip;
-    if (type == SourcePackage)
+    if (package->type == SourcePackage)
         component += TypeBottom;
     mxf_write_uuid(pb, component, st->index);
 }
 
-static void mxf_write_timecode_component(AVFormatContext *s, AVStream *st, enum MXFMetadataSetType type)
+static void mxf_write_timecode_component(AVFormatContext *s, AVStream *st, MXFPackage *package)
 {
     MXFContext *mxf = s->priv_data;
     AVIOContext *pb = s->pb;
@@ -943,7 +951,7 @@  static void mxf_write_timecode_component(AVFormatContext *s, AVStream *st, enum
 
     // UID
     mxf_write_local_tag(pb, 16, 0x3C0A);
-    mxf_write_uuid(pb, type == MaterialPackage ? TimecodeComponent :
+    mxf_write_uuid(pb, package->type == MaterialPackage ? TimecodeComponent :
                    TimecodeComponent + TypeBottom, st->index);
 
     mxf_write_common_fields(s, st);
@@ -961,7 +969,7 @@  static void mxf_write_timecode_component(AVFormatContext *s, AVStream *st, enum
     avio_w8(pb, !!(mxf->tc.flags & AV_TIMECODE_FLAG_DROPFRAME));
 }
 
-static void mxf_write_structural_component(AVFormatContext *s, AVStream *st, enum MXFMetadataSetType type)
+static void mxf_write_structural_component(AVFormatContext *s, AVStream *st, MXFPackage *package)
 {
     AVIOContext *pb = s->pb;
     int i;
@@ -972,7 +980,7 @@  static void mxf_write_structural_component(AVFormatContext *s, AVStream *st, enu
 
     // write uid
     mxf_write_local_tag(pb, 16, 0x3C0A);
-    mxf_write_uuid(pb, type == MaterialPackage ? SourceClip: SourceClip + TypeBottom, st->index);
+    mxf_write_uuid(pb, package->type == MaterialPackage ? SourceClip: SourceClip + TypeBottom, st->index);
 
     PRINT_KEY(s, "structural component uid", pb->buf_ptr - 16);
     mxf_write_common_fields(s, st);
@@ -983,7 +991,7 @@  static void mxf_write_structural_component(AVFormatContext *s, AVStream *st, enu
 
     // write source package uid, end of the reference
     mxf_write_local_tag(pb, 32, 0x1101);
-    if (type == SourcePackage) {
+    if (package->type == SourcePackage) {
         for (i = 0; i < 4; i++)
             avio_wb64(pb, 0);
     } else
@@ -991,7 +999,7 @@  static void mxf_write_structural_component(AVFormatContext *s, AVStream *st, enu
 
     // write source track id
     mxf_write_local_tag(pb, 4, 0x1102);
-    if (type == SourcePackage)
+    if (package->type == SourcePackage)
         avio_wb32(pb, 0);
     else
         avio_wb32(pb, st->index+2);
@@ -1321,15 +1329,15 @@  static int mxf_write_user_comments(AVFormatContext *s, const AVDictionary *m)
     return count;
 }
 
-static void mxf_write_package(AVFormatContext *s, enum MXFMetadataSetType type, const char *package_name)
+static void mxf_write_package(AVFormatContext *s, MXFPackage *package)
 {
     MXFContext *mxf = s->priv_data;
     AVIOContext *pb = s->pb;
     int i, track_count = s->nb_streams+1;
-    int name_size = mxf_utf16_local_tag_length(package_name);
+    int name_size = mxf_utf16_local_tag_length(package->name);
     int user_comment_count = 0;
 
-    if (type == MaterialPackage) {
+    if (package->type == MaterialPackage) {
         if (mxf->store_user_comments)
             user_comment_count = mxf_write_user_comments(s, s->metadata);
         mxf_write_metadata_key(pb, 0x013600);
@@ -1343,18 +1351,18 @@  static void mxf_write_package(AVFormatContext *s, enum MXFMetadataSetType type,
 
     // write uid
     mxf_write_local_tag(pb, 16, 0x3C0A);
-    mxf_write_uuid(pb, type, 0);
-    av_log(s, AV_LOG_DEBUG, "package type:%d\n", type);
+    mxf_write_uuid(pb, package->type, package->instance);
+    av_log(s, AV_LOG_DEBUG, "package type:%d\n", package->type);
     PRINT_KEY(s, "package uid", pb->buf_ptr - 16);
 
     // write package umid
     mxf_write_local_tag(pb, 32, 0x4401);
-    mxf_write_umid(s, type == SourcePackage);
+    mxf_write_umid(s, package->type == SourcePackage);
     PRINT_KEY(s, "package umid second part", pb->buf_ptr - 16);
 
     // package name
     if (name_size)
-        mxf_write_local_tag_utf16(pb, 0x4402, package_name);
+        mxf_write_local_tag_utf16(pb, 0x4402, package->name);
 
     // package creation date
     mxf_write_local_tag(pb, 8, 0x4405);
@@ -1367,10 +1375,10 @@  static void mxf_write_package(AVFormatContext *s, enum MXFMetadataSetType type,
     // write track refs
     mxf_write_local_tag(pb, track_count*16 + 8, 0x4403);
     mxf_write_refs_count(pb, track_count);
-    mxf_write_uuid(pb, type == MaterialPackage ? Track :
+    mxf_write_uuid(pb, package->type == MaterialPackage ? Track :
                    Track + TypeBottom, -1); // timecode track
     for (i = 0; i < s->nb_streams; i++)
-        mxf_write_uuid(pb, type == MaterialPackage ? Track : Track + TypeBottom, i);
+        mxf_write_uuid(pb, package->type == MaterialPackage ? Track : Track + TypeBottom, i);
 
     // write user comment refs
     if (mxf->store_user_comments) {
@@ -1381,7 +1389,7 @@  static void mxf_write_package(AVFormatContext *s, enum MXFMetadataSetType type,
     }
 
     // write multiple descriptor reference
-    if (type == SourcePackage) {
+    if (package->type == SourcePackage) {
         mxf_write_local_tag(pb, 16, 0x4701);
         if (s->nb_streams > 1) {
             mxf_write_uuid(pb, MultipleDescriptor, 0);
@@ -1391,17 +1399,17 @@  static void mxf_write_package(AVFormatContext *s, enum MXFMetadataSetType type,
     }
 
     // write timecode track
-    mxf_write_track(s, mxf->timecode_track, type);
-    mxf_write_sequence(s, mxf->timecode_track, type);
-    mxf_write_timecode_component(s, mxf->timecode_track, type);
+    mxf_write_track(s, mxf->timecode_track, package);
+    mxf_write_sequence(s, mxf->timecode_track, package);
+    mxf_write_timecode_component(s, mxf->timecode_track, package);
 
     for (i = 0; i < s->nb_streams; i++) {
         AVStream *st = s->streams[i];
-        mxf_write_track(s, st, type);
-        mxf_write_sequence(s, st, type);
-        mxf_write_structural_component(s, st, type);
+        mxf_write_track(s, st, package);
+        mxf_write_sequence(s, st, package);
+        mxf_write_structural_component(s, st, package);
 
-        if (type == SourcePackage) {
+        if (package->type == SourcePackage) {
             MXFStreamContext *sc = st->priv_data;
             mxf_essence_container_uls[sc->index].write_desc(s, st);
         }
@@ -1432,23 +1440,26 @@  static int mxf_write_essence_container_data(AVFormatContext *s)
 
 static int mxf_write_header_metadata_sets(AVFormatContext *s)
 {
-    const char *material_package_name = NULL;
-    const char *file_package_name = NULL;
     AVDictionaryEntry *entry = NULL;
     AVStream *st = NULL;
     int i;
 
+    MXFPackage packages[2] = {};
+    int package_count = 2;
+    packages[0].type = MaterialPackage;
+    packages[1].type = SourcePackage;
+
     if (entry = av_dict_get(s->metadata, "material_package_name", NULL, 0))
-       material_package_name = entry->value;
+       packages[0].name = entry->value;
 
     if (entry = av_dict_get(s->metadata, "file_package_name", NULL, 0)) {
-        file_package_name = entry->value;
+        packages[1].name = entry->value;
     } else {
         /* check if any of the streams contain a file_package_name */
         for (i = 0; i < s->nb_streams; i++) {
             st = s->streams[i];
             if (entry = av_dict_get(st->metadata, "file_package_name", NULL, 0)) {
-                file_package_name = entry->value;
+                packages[1].name = entry->value;
                 break;
             }
         }
@@ -1456,9 +1467,9 @@  static int mxf_write_header_metadata_sets(AVFormatContext *s)
 
     mxf_write_preface(s);
     mxf_write_identification(s);
-    mxf_write_content_storage(s);
-    mxf_write_package(s, MaterialPackage, material_package_name);
-    mxf_write_package(s, SourcePackage, file_package_name);
+    mxf_write_content_storage(s, packages, package_count);
+    for (i = 0; i < package_count; i++)
+        mxf_write_package(s, &packages[i]);
     mxf_write_essence_container_data(s);
     return 0;
 }