Re: [FFmpeg-devel] [PATCH] libavformat/mxfdec: export user comments metadata

2015-03-13 Thread Mark Reid
On Fri, Mar 13, 2015 at 3:57 AM, Tomas Härdin tomas.har...@codemill.se
wrote:

 On Fri, 2015-03-06 at 13:24 -0800, Mark Reid wrote:
  +static int mxf_read_indirect_value(void *arg, AVIOContext *pb, int size)
  +{
  +MXFTaggedValue *tagged_value = arg;
  +uint8_t key[17];
  +
  +if (size = 17)
  +return 0;
  +
  +avio_read(pb, key, 17);

 Really Avid, 17 byte keys? OK..


Its not really a 17 byte key, the first byte is the endianness, (0x4c == le
and 0x42 == be). I added the byte to the front of the key because I thought
it made identifying the indirect type logic simpler.


  +/* TODO: handle other types of of indirect values */
  +if (memcmp(key, mxf_indirect_value_utf16le, 17) == 0) {
  +return mxf_read_utf16le_string(pb, size - 17,
 tagged_value-value);
  +} else if (memcmp(key, mxf_indirect_value_utf16be, 17) == 0) {
  +return mxf_read_utf16be_string(pb, size - 17,
 tagged_value-value);
  +}
  +return 0;
  +}

  +static int mxf_parse_package_comments(MXFContext *mxf, AVDictionary
 **pm, MXFPackage *package)
  +{
  +MXFTaggedValue *tag;
  +int size, i;
  +const char *prefix = comment_;
  +char *key = NULL;
  +
  +for (i = 0; i  package-comment_count; i++) {
  +tag = mxf_resolve_strong_ref(mxf, package-comment_refs[i],
 TaggedValue);
  +if (!tag || !tag-name || !tag-value)
  +continue;
  +
  +size = strlen(prefix) + strlen(tag-name) + 1;
  +key = av_mallocz(size);
  +if (!key)
  +return AVERROR(ENOMEM);
  +
  +strcpy(key, prefix);
  +strlcat(key, tag-name, size);

 snprintf() would make this one line only, or use av_strlcat() like
 Michael suggests. Come to think of it, I'm not sure snprintf() exists on
 all platforms..


I see snprintf being used quite a lot used throughout the project, I'll try
using that instead,


  +av_dict_set(pm, key, tag-value, AV_DICT_DONT_STRDUP_KEY);
  +}
  +return 0;
  +}

 Looks good overall, even if I wasn't aware of this Avid feature before.


great, I'm hoping to add setting these metadata keys to mxfenc as well,
because these tags show up in Avid bins as new columns.
I'll submit a new patch fixing the strlcat, thanks for taking the time to
review.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] libavformat/mxfdec: export user comments metadata

2015-03-13 Thread Tomas Härdin
On Fri, 2015-03-06 at 13:24 -0800, Mark Reid wrote:
 +static int mxf_read_indirect_value(void *arg, AVIOContext *pb, int size)
 +{
 +MXFTaggedValue *tagged_value = arg;
 +uint8_t key[17];
 +
 +if (size = 17)
 +return 0;
 +
 +avio_read(pb, key, 17);

Really Avid, 17 byte keys? OK..

 +/* TODO: handle other types of of indirect values */
 +if (memcmp(key, mxf_indirect_value_utf16le, 17) == 0) {
 +return mxf_read_utf16le_string(pb, size - 17, tagged_value-value);
 +} else if (memcmp(key, mxf_indirect_value_utf16be, 17) == 0) {
 +return mxf_read_utf16be_string(pb, size - 17, tagged_value-value);
 +}
 +return 0;
 +}

 +static int mxf_parse_package_comments(MXFContext *mxf, AVDictionary **pm, 
 MXFPackage *package)
 +{
 +MXFTaggedValue *tag;
 +int size, i;
 +const char *prefix = comment_;
 +char *key = NULL;
 +
 +for (i = 0; i  package-comment_count; i++) {
 +tag = mxf_resolve_strong_ref(mxf, package-comment_refs[i], 
 TaggedValue);
 +if (!tag || !tag-name || !tag-value)
 +continue;
 +
 +size = strlen(prefix) + strlen(tag-name) + 1;
 +key = av_mallocz(size);
 +if (!key)
 +return AVERROR(ENOMEM);
 +
 +strcpy(key, prefix);
 +strlcat(key, tag-name, size);

snprintf() would make this one line only, or use av_strlcat() like
Michael suggests. Come to think of it, I'm not sure snprintf() exists on
all platforms..

 +av_dict_set(pm, key, tag-value, AV_DICT_DONT_STRDUP_KEY);
 +}
 +return 0;
 +}

Looks good overall, even if I wasn't aware of this Avid feature before.

/Tomas


signature.asc
Description: This is a digitally signed message part
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] libavformat/mxfdec: export user comments metadata

2015-03-06 Thread Mark Reid
---
 libavformat/mxf.h|  1 +
 libavformat/mxfdec.c | 96 +---
 2 files changed, 93 insertions(+), 4 deletions(-)

diff --git a/libavformat/mxf.h b/libavformat/mxf.h
index d9e17c6..71a4084 100644
--- a/libavformat/mxf.h
+++ b/libavformat/mxf.h
@@ -47,6 +47,7 @@ enum MXFMetadataSetType {
 EssenceContainerData,
 TypeBottom,// add metadata type before this
 EssenceGroup,
+TaggedValue,
 };
 
 enum MXFFrameLayout {
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 2e8dd05..970df33 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -144,6 +144,13 @@ typedef struct {
 typedef struct {
 UID uid;
 enum MXFMetadataSetType type;
+char *name;
+char *value;
+} MXFTaggedValue;
+
+typedef struct {
+UID uid;
+enum MXFMetadataSetType type;
 MXFSequence *sequence; /* mandatory, and only one */
 UID sequence_ref;
 int track_id;
@@ -206,6 +213,8 @@ typedef struct MXFPackage {
 MXFDescriptor *descriptor; /* only one */
 UID descriptor_ref;
 char *name;
+UID *comment_refs;
+int comment_count;
 } MXFPackage;
 
 typedef struct MXFMetadataSet {
@@ -282,6 +291,8 @@ static const uint8_t mxf_random_index_pack_key[]   
= { 0x06,0x0e,0x2b,0x
 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_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,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x01,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
 };
+static const uint8_t mxf_indirect_value_utf16be[]  = { 
0x42,0x01,0x10,0x02,0x00,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01
 };
 
 #define IS_KLV_KEY(x, y) (!memcmp(x, y, sizeof(y)))
 
@@ -306,6 +317,10 @@ static void mxf_free_metadataset(MXFMetadataSet **ctx, int 
freectx)
 av_freep(((MXFPackage *)*ctx)-tracks_refs);
 av_freep(((MXFPackage *)*ctx)-name);
 break;
+case TaggedValue:
+av_freep(((MXFTaggedValue *)*ctx)-name);
+av_freep(((MXFTaggedValue *)*ctx)-value);
+break;
 case IndexTableSegment:
 seg = (MXFIndexTableSegment *)*ctx;
 av_freep(seg-temporal_offset_entries);
@@ -803,7 +818,7 @@ static int mxf_read_essence_group(void *arg, AVIOContext 
*pb, int tag, int size,
 return 0;
 }
 
-static int mxf_read_utf16_string(AVIOContext *pb, int size, char** str)
+static inline int mxf_read_utf16_string(AVIOContext *pb, int size, char** str, 
int be)
 {
 int ret;
 size_t buf_size;
@@ -816,7 +831,12 @@ static int mxf_read_utf16_string(AVIOContext *pb, int 
size, char** str)
 if (!*str)
 return AVERROR(ENOMEM);
 
-if ((ret = avio_get_str16be(pb, size, *str, buf_size))  0) {
+if (be)
+ret = avio_get_str16be(pb, size, *str, buf_size);
+else
+ret = avio_get_str16le(pb, size, *str, buf_size);
+
+if (ret  0) {
 av_freep(str);
 return ret;
 }
@@ -824,6 +844,15 @@ static int mxf_read_utf16_string(AVIOContext *pb, int 
size, char** str)
 return ret;
 }
 
+#define READ_STR16(type, big_endian)   
\
+static int mxf_read_utf16 ## type ##_string(AVIOContext *pb, int size, char** 
str) \
+{  
\
+return mxf_read_utf16_string(pb, size, str, big_endian);   
\
+}
+READ_STR16(be, 1)
+READ_STR16(le, 0)
+#undef READ_STR16
+
 static int mxf_read_package(void *arg, AVIOContext *pb, int tag, int size, UID 
uid, int64_t klv_offset)
 {
 MXFPackage *package = arg;
@@ -840,7 +869,10 @@ static int mxf_read_package(void *arg, AVIOContext *pb, 
int tag, int size, UID u
 avio_read(pb, package-descriptor_ref, 16);
 break;
 case 0x4402:
-return mxf_read_utf16_string(pb, size, package-name);
+return mxf_read_utf16be_string(pb, size, package-name);
+case 0x4406:
+return mxf_read_strong_ref_array(pb, package-comment_refs,
+ package-comment_count);
 }
 return 0;
 }
@@ -1012,6 +1044,36 @@ static int mxf_read_generic_descriptor(void *arg, 
AVIOContext *pb, int tag, int
 return 0;
 }
 
+static int mxf_read_indirect_value(void *arg, AVIOContext *pb, int size)
+{
+MXFTaggedValue *tagged_value = arg;
+uint8_t key[17];
+
+if (size = 17)
+return 0;
+
+avio_read(pb, key, 17);
+/* TODO: handle other types of of indirect values */
+if (memcmp(key, mxf_indirect_value_utf16le, 17) == 0) {
+return mxf_read_utf16le_string(pb, size - 17, 

[FFmpeg-devel] [PATCH] libavformat/mxfdec: export user comments metadata

2015-03-06 Thread Mark Reid
Hi
This patch enable reading of tagged values from mxf files and export
the user comment tags as metadata. The tags can have arbitrary names
and values. values can also have different types.  I have only
added support for the utf16be and utf16le types. Avid for some reason
encodes tag indirect values as a utf16le strings. This patch only
exports the user comments tags, but there other tagged
values being read that aren't being exported. some of these tags also
have subtags which I'm also ignoring as well.

here is a sample mxf file that contains a bunch of user comments
https://dl.dropboxusercontent.com/u/170952/ffmpeg_samples/tagged_values.mxf

ffprobe tagged_values.mxf should show

comment_UNC Path: /Users/mark/Dev/ffmpeg/testsrc.mov
comment_example 3: 2
comment_example 1: value
comment_example 2: value2
comment_example 4: Lorem ipsum dolor
comment_example_5: £20.00
comment_TapeID  : TestSrC
comment_example 6: 100Ω
comment_example 7: ©2014

I'm currently prefixing all the user comments with comment_ to separate them
from other metadata keys. The keys look kinda ugly and can have spaces,
if someone has a better suggestion I'd love to here it.

Mark Reid (1):
  libavformat/mxfdec: export user comments metadata

 libavformat/mxf.h|  1 +
 libavformat/mxfdec.c | 96 +---
 2 files changed, 93 insertions(+), 4 deletions(-)

-- 
2.2.1

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


Re: [FFmpeg-devel] [PATCH] libavformat/mxfdec: export user comments metadata

2015-03-06 Thread Michael Niedermayer
On Fri, Mar 06, 2015 at 01:24:35PM -0800, Mark Reid wrote:
 ---
  libavformat/mxf.h|  1 +
  libavformat/mxfdec.c | 96 
 +---
  2 files changed, 93 insertions(+), 4 deletions(-)
[...]
 @@ -1630,6 +1692,30 @@ static MXFStructuralComponent* 
 mxf_resolve_sourceclip(MXFContext *mxf, UID *stro
  return NULL;
  }
  
 +static int mxf_parse_package_comments(MXFContext *mxf, AVDictionary **pm, 
 MXFPackage *package)
 +{
 +MXFTaggedValue *tag;
 +int size, i;
 +const char *prefix = comment_;
 +char *key = NULL;
 +
 +for (i = 0; i  package-comment_count; i++) {
 +tag = mxf_resolve_strong_ref(mxf, package-comment_refs[i], 
 TaggedValue);
 +if (!tag || !tag-name || !tag-value)
 +continue;
 +
 +size = strlen(prefix) + strlen(tag-name) + 1;
 +key = av_mallocz(size);
 +if (!key)
 +return AVERROR(ENOMEM);
 +
 +strcpy(key, prefix);

 +strlcat(key, tag-name, size);

not available on all platforms, please use av_strlcat()

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

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel