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

2017-11-30 Thread Matthias Troffaes 
Dear Mark,

On Thu, Nov 30, 2017 at 4:53 AM, Mark Reid  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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2017-11-29 Thread Mark Reid 
On Wed, Nov 29, 2017 at 8:19 PM, Mark Reid  wrote:

>
>
> On Wed, Nov 29, 2017 at 12:56 PM, Matthias Troffaes <
> matthias.troff...@gmail.com> wrote:
>
>> Dear Mark,
>>
>> On Wed, Nov 29, 2017 at 4:11 AM, Mark Reid  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
>>
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2017-11-29 Thread Mark Reid 
On Wed, Nov 29, 2017 at 12:56 PM, Matthias Troffaes <
matthias.troff...@gmail.com> wrote:

> Dear Mark,
>
> On Wed, Nov 29, 2017 at 4:11 AM, Mark Reid  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
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2017-11-29 Thread Matthias Troffaes 
Dear Mark,

On Wed, Nov 29, 2017 at 4:11 AM, Mark Reid  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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2017-11-29 Thread Tomas Härdin 

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 = [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 = [2];
+package_count = 3;
+}
+
  mxf_write_preface(s);
  mxf_write_identification(s);
  mxf_write_content_storage(s, packages, package_count);


Looks OK

/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel