Hi there, Here is a patch to extract the starting timecode from mxf files. Feel free to comment, Matthieu
2012/1/11 Matthieu Bouron <matthieu.bouron@gmail.com>:
Hi there,
Here is a patch to extract the starting timecode from mxf files.
Feel free to comment, Matthieu
ping
On Mon, Jan 16, 2012 at 11:10:25AM +0100, Matthieu Bouron wrote:
2012/1/11 Matthieu Bouron <matthieu.bouron@gmail.com>:
Hi there,
Here is a patch to extract the starting timecode from mxf files.
Feel free to comment, Matthieu
ping
baptiste? tomas? -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 1 "Used only once" - "Some unspecified defect prevented a second use" "In good condition" - "Can be repaird by experienced expert" "As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
Matthieu Bouron <matthieu.bouron@gmail.com> wrote:-
2012/1/11 Matthieu Bouron <matthieu.bouron@gmail.com>:
Hi there,
Here is a patch to extract the starting timecode from mxf files.
Feel free to comment, Matthieu
ping
Without wishing to comment on the code, which is not my area of expertise. This works for me on *some* of my MXF sample files but not all. It seems fine on XDCAM-HD Clips, but not on the BBC's "Preservation Master" format. Which I know has timecode as "archive_mxf_info" from:- http://sourceforge.net/p/bmxlib/home/Home/ extracts it. -- Tim
2012/1/16 Tim Nicholson <nichot20@yahoo.com>:
Matthieu Bouron <matthieu.bouron@gmail.com> wrote:-
2012/1/11 Matthieu Bouron <matthieu.bouron@gmail.com>:
Hi there,
Here is a patch to extract the starting timecode from mxf files.
Feel free to comment, Matthieu
ping
Without wishing to comment on the code, which is not my area of expertise. This works for me on *some* of my MXF sample files but not all.
It seems fine on XDCAM-HD Clips, but not on the BBC's "Preservation Master" format. Which I know has timecode as "archive_mxf_info" from:-
http://sourceforge.net/p/bmxlib/home/Home/
extracts it.
The starting timecode could be present in the structural metadata and the system metadata. This patch only support extraction from structural metadata; maybe this is why the timecode is not detected from the BBC's "Preversation Master" files. Some sample would help me determine this. Can you provide some ? Thanks
-- Tim _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 16/01/12 17:12, Matthieu Bouron wrote:
The starting timecode could be present in the structural metadata and the system metadata. This patch only support extraction from structural metadata; maybe this is why the timecode is not detected from the BBC's "Preversation Master" files. Some sample would help me determine this. Can you provide some ?
I will try and generate some small ones (it will have to be tomorrow morning now) Where would you like them? on the mplayer ftp?
Thanks
-- Tim
2012/1/16 Tim Nicholson <nichot20@yahoo.com>:
On 16/01/12 17:12, Matthieu Bouron wrote:
The starting timecode could be present in the structural metadata and the system metadata. This patch only support extraction from structural metadata; maybe this is why the timecode is not detected from the BBC's "Preversation Master" files. Some sample would help me determine this. Can you provide some ?
I will try and generate some small ones (it will have to be tomorrow morning now) Where would you like them? on the mplayer ftp?
Mplayer ftp sounds good but i don't have access to it. As an temporary alternative, can you upload these samples to 91.121.134.112: guest/samples ? Thanks
Thanks
-- Tim _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 17/01/12 09:27, Matthieu Bouron wrote:
2012/1/16 Tim Nicholson <nichot20@yahoo.com>:
On 16/01/12 17:12, Matthieu Bouron wrote:
The starting timecode could be present in the structural metadata and the system metadata. This patch only support extraction from structural metadata; maybe this is why the timecode is not detected from the BBC's "Preversation Master" files. Some sample would help me determine this. Can you provide some ?
I will try and generate some small ones (it will have to be tomorrow morning now) Where would you like them? on the mplayer ftp?
Mplayer ftp sounds good but i don't have access to it. As an temporary alternative, can you upload these samples to 91.121.134.112: guest/samples ?
It doesn't seem to accept anonymous logins.... -- Tim
2012/1/17 Tim Nicholson <nichot20@yahoo.com>:
On 17/01/12 09:27, Matthieu Bouron wrote:
2012/1/16 Tim Nicholson <nichot20@yahoo.com>:
On 16/01/12 17:12, Matthieu Bouron wrote:
The starting timecode could be present in the structural metadata and the system metadata. This patch only support extraction from structural metadata; maybe this is why the timecode is not detected from the BBC's "Preversation Master" files. Some sample would help me determine this. Can you provide some ?
I will try and generate some small ones (it will have to be tomorrow morning now) Where would you like them? on the mplayer ftp?
Mplayer ftp sounds good but i don't have access to it. As an temporary alternative, can you upload these samples to 91.121.134.112: guest/samples ?
It doesn't seem to accept anonymous logins.... The account is guest, password: samples
-- Tim _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 17/01/12 10:27, Matthieu Bouron wrote:
2012/1/17 Tim Nicholson <nichot20@yahoo.com>:
On 17/01/12 09:27, Matthieu Bouron wrote:
2012/1/16 Tim Nicholson <nichot20@yahoo.com>:
On 16/01/12 17:12, Matthieu Bouron wrote:
The starting timecode could be present in the structural metadata and the system metadata. This patch only support extraction from structural metadata; maybe this is why the timecode is not detected from the BBC's "Preversation Master" files. Some sample would help me determine this. Can you provide some ?
I will try and generate some small ones (it will have to be tomorrow morning now) Where would you like them? on the mplayer ftp?
Mplayer ftp sounds good but i don't have access to it. As an temporary alternative, can you upload these samples to 91.121.134.112: guest/samples ?
Sorry I read that as the directory not the login......
It doesn't seem to accept anonymous logins....
The account is guest, password: samples
Uploading now a 5sec clip -- Tim
On Wed, 2012-01-11 at 00:24 +0100, Matthieu Bouron wrote:
Hi there,
Here is a patch to extract the starting timecode from mxf files.
+static int mxf_read_timecode_component(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) +{ + MXFTimecodeComponent *mxf_timecode = arg; + switch(tag) { + case 0x1501: + mxf_timecode->tc.start = avio_rb64(pb); + break; + case 0x1502: + mxf_timecode->tc.rate = (AVRational){avio_rb16(pb), 1}; + break; + case 0x1503: + mxf_timecode->tc.drop = avio_r8(pb); + break; + } + return 0; +}
Maybe you should read the rest of the structural component fields too, like Duration? Might be useful later, but perhaps not relevant to this patch so no biggie.
@@ -1249,9 +1285,17 @@ static int mxf_parse_structural_metadata(MXFContext *mxf) continue; }
+ for (j = 0; j < material_track->sequence->structural_components_count; j++) {
You should probably use the SourcePackage's timecode track, since we play all the essence (MP might point to only a subset of the media). That or warn if StartPosition != 0, which would be simpler.. Or we could obey StartPosition :)
+ component = mxf_resolve_strong_ref(mxf, &material_track->sequence->structural_components_refs[j], TimecodeComponent); + if (!component) + continue; + + MXFTimecodeComponent *mxf_tc = (MXFTimecodeComponent*)component; + mxf_add_timecode_metadata(&mxf->fc->metadata, "timecode", &mxf_tc->tc);
break; in case there are more than one track. I think the first one is the proper one to use. Issuing a warning in case there's more than one track may be proper. Looks OK overall. /Tomas
2012/1/16 Tomas Härdin <tomas.hardin@codemill.se>:
On Wed, 2012-01-11 at 00:24 +0100, Matthieu Bouron wrote:
Hi there,
Here is a patch to extract the starting timecode from mxf files.
+static int mxf_read_timecode_component(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) +{ + MXFTimecodeComponent *mxf_timecode = arg; + switch(tag) { + case 0x1501: + mxf_timecode->tc.start = avio_rb64(pb); + break; + case 0x1502: + mxf_timecode->tc.rate = (AVRational){avio_rb16(pb), 1}; + break; + case 0x1503: + mxf_timecode->tc.drop = avio_r8(pb); + break; + } + return 0; +}
Maybe you should read the rest of the structural component fields too, like Duration? Might be useful later, but perhaps not relevant to this patch so no biggie.
@@ -1249,9 +1285,17 @@ static int mxf_parse_structural_metadata(MXFContext *mxf) continue; }
+ for (j = 0; j < material_track->sequence->structural_components_count; j++) {
You should probably use the SourcePackage's timecode track, since we play all the essence (MP might point to only a subset of the media). That or warn if StartPosition != 0, which would be simpler.. Or we could obey StartPosition :)
+ component = mxf_resolve_strong_ref(mxf, &material_track->sequence->structural_components_refs[j], TimecodeComponent); + if (!component) + continue; + + MXFTimecodeComponent *mxf_tc = (MXFTimecodeComponent*)component; + mxf_add_timecode_metadata(&mxf->fc->metadata, "timecode", &mxf_tc->tc);
break; in case there are more than one track. I think the first one is the proper one to use. Issuing a warning in case there's more than one track may be proper.
Looks OK overall.
Thanks for the review, i ll update the patch ASAP.
/Tomas
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
2012/1/16 Tomas Härdin <tomas.hardin@codemill.se>:
On Wed, 2012-01-11 at 00:24 +0100, Matthieu Bouron wrote:
Hi there,
Here is a patch to extract the starting timecode from mxf files.
+static int mxf_read_timecode_component(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) +{ + MXFTimecodeComponent *mxf_timecode = arg; + switch(tag) { + case 0x1501: + mxf_timecode->tc.start = avio_rb64(pb); + break; + case 0x1502: + mxf_timecode->tc.rate = (AVRational){avio_rb16(pb), 1}; + break; + case 0x1503: + mxf_timecode->tc.drop = avio_r8(pb); + break; + } + return 0; +}
Maybe you should read the rest of the structural component fields too, like Duration? Might be useful later, but perhaps not relevant to this patch so no biggie.
@@ -1249,9 +1285,17 @@ static int mxf_parse_structural_metadata(MXFContext *mxf) continue; }
+ for (j = 0; j < material_track->sequence->structural_components_count; j++) {
You should probably use the SourcePackage's timecode track, since we play all the essence (MP might point to only a subset of the media). That or warn if StartPosition != 0, which would be simpler.. Or we could obey StartPosition :)
I don't think reading the starting timecode track from source package is a good idea since only timecode track from material package represent the playout timecode.
+ component = mxf_resolve_strong_ref(mxf, &material_track->sequence->structural_components_refs[j], TimecodeComponent); + if (!component) + continue; + + MXFTimecodeComponent *mxf_tc = (MXFTimecodeComponent*)component; + mxf_add_timecode_metadata(&mxf->fc->metadata, "timecode", &mxf_tc->tc);
break; in case there are more than one track. I think the first one is the proper one to use. Issuing a warning in case there's more than one track may be proper.
done. Patch updated using the new timecode API. I also check if a timecode component is not directly strong referenced by a timecode track (no sequence). It seems to be the case in the BBC "preservation files". Anyway is this not a violation of the spec ?
Looks OK overall.
/Tomas
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Thu, Feb 23, 2012 at 06:00:11PM +0100, Matthieu Bouron wrote: [...]
+static int mxf_read_timecode_component(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) +{ + MXFTimecodeComponent *mxf_timecode = arg; + switch(tag) { + case 0x1501: + mxf_timecode->start_frame = avio_rb64(pb); + break; + case 0x1502: + mxf_timecode->rate = (AVRational){avio_rb16(pb), 1}; + break; + case 0x1503: + mxf_timecode->drop_frame = avio_r8(pb); + break; + } + return 0; +} + static int mxf_read_track(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) { MXFTrack *track = arg; @@ -1233,6 +1260,14 @@ finish_decoding_index: return ret; }
+static int mxf_add_timecode_metadata(AVDictionary **pm, const char *key, AVTimecode *tc) +{ + char buf[16];
AV_TIMECODE_STR_SIZE instead of 16
+ av_dict_set(pm, key, av_timecode_make_string(tc, buf, 0), 0); + + return 0; +} + static int mxf_parse_structural_metadata(MXFContext *mxf) { MXFPackage *material_package = NULL; @@ -1257,24 +1292,44 @@ static int mxf_parse_structural_metadata(MXFContext *mxf) MXFTrack *temp_track = NULL; MXFDescriptor *descriptor = NULL; MXFStructuralComponent *component = NULL; + MXFTimecodeComponent *mxf_tc = NULL; UID *essence_container_ul = NULL; const MXFCodecUL *codec_ul = NULL; const MXFCodecUL *container_ul = NULL; AVStream *st; + AVTimecode tc;
if (!(material_track = mxf_resolve_strong_ref(mxf, &material_package->tracks_refs[i], Track))) { av_log(mxf->fc, AV_LOG_ERROR, "could not resolve material track strong ref\n"); continue; }
+ if ((component = mxf_resolve_strong_ref(mxf, &material_track->sequence_ref, TimecodeComponent))) { + mxf_tc = (MXFTimecodeComponent*)component; + if (av_timecode_init(&tc, mxf_tc->rate, mxf_tc->drop_frame == 1, mxf_tc->start_frame, mxf) == 0) {
Better use a variable like flags = mxf_tc->drop_frame == 1 ? AV_TIMECODE_FLAG_DROPFRAME : 0, instead of assuming the value of the flag.
+ mxf_add_timecode_metadata(&mxf->fc->metadata, "timecode", &tc); + } + } + if (!(material_track->sequence = mxf_resolve_strong_ref(mxf, &material_track->sequence_ref, Sequence))) { av_log(mxf->fc, AV_LOG_ERROR, "could not resolve material track sequence strong ref\n"); continue; }
+ for (j = 0; j < material_track->sequence->structural_components_count; j++) { + component = mxf_resolve_strong_ref(mxf, &material_track->sequence->structural_components_refs[j], TimecodeComponent); + if (!component) + continue; + + mxf_tc = (MXFTimecodeComponent*)component; + if (av_timecode_init(&tc, mxf_tc->rate, mxf_tc->drop_frame == 1, mxf_tc->start_frame, mxf) == 0) {
Same as above
+ mxf_add_timecode_metadata(&mxf->fc->metadata, "timecode", &tc);
It looks like you are replacing the timecode metadata, I guess it's desired? BTW, what about the SMPTE timecode in the "system item"? Also, you might want to update doc/general.texi if the feature is complete. [...] -- Clément B.
2012/2/23 Clément Bœsch <ubitux@gmail.com>:
On Thu, Feb 23, 2012 at 06:00:11PM +0100, Matthieu Bouron wrote: [...]
+static int mxf_read_timecode_component(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) +{ + MXFTimecodeComponent *mxf_timecode = arg; + switch(tag) { + case 0x1501: + mxf_timecode->start_frame = avio_rb64(pb); + break; + case 0x1502: + mxf_timecode->rate = (AVRational){avio_rb16(pb), 1}; + break; + case 0x1503: + mxf_timecode->drop_frame = avio_r8(pb); + break; + } + return 0; +} + static int mxf_read_track(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) { MXFTrack *track = arg; @@ -1233,6 +1260,14 @@ finish_decoding_index: return ret; }
+static int mxf_add_timecode_metadata(AVDictionary **pm, const char *key, AVTimecode *tc) +{ + char buf[16];
AV_TIMECODE_STR_SIZE instead of 16
+ av_dict_set(pm, key, av_timecode_make_string(tc, buf, 0), 0); + + return 0; +} + static int mxf_parse_structural_metadata(MXFContext *mxf) { MXFPackage *material_package = NULL; @@ -1257,24 +1292,44 @@ static int mxf_parse_structural_metadata(MXFContext *mxf) MXFTrack *temp_track = NULL; MXFDescriptor *descriptor = NULL; MXFStructuralComponent *component = NULL; + MXFTimecodeComponent *mxf_tc = NULL; UID *essence_container_ul = NULL; const MXFCodecUL *codec_ul = NULL; const MXFCodecUL *container_ul = NULL; AVStream *st; + AVTimecode tc;
if (!(material_track = mxf_resolve_strong_ref(mxf, &material_package->tracks_refs[i], Track))) { av_log(mxf->fc, AV_LOG_ERROR, "could not resolve material track strong ref\n"); continue; }
+ if ((component = mxf_resolve_strong_ref(mxf, &material_track->sequence_ref, TimecodeComponent))) { + mxf_tc = (MXFTimecodeComponent*)component; + if (av_timecode_init(&tc, mxf_tc->rate, mxf_tc->drop_frame == 1, mxf_tc->start_frame, mxf) == 0) {
Better use a variable like flags = mxf_tc->drop_frame == 1 ? AV_TIMECODE_FLAG_DROPFRAME : 0, instead of assuming the value of the flag.
+ mxf_add_timecode_metadata(&mxf->fc->metadata, "timecode", &tc); + } + } + if (!(material_track->sequence = mxf_resolve_strong_ref(mxf, &material_track->sequence_ref, Sequence))) { av_log(mxf->fc, AV_LOG_ERROR, "could not resolve material track sequence strong ref\n"); continue; }
+ for (j = 0; j < material_track->sequence->structural_components_count; j++) { + component = mxf_resolve_strong_ref(mxf, &material_track->sequence->structural_components_refs[j], TimecodeComponent); + if (!component) + continue; + + mxf_tc = (MXFTimecodeComponent*)component; + if (av_timecode_init(&tc, mxf_tc->rate, mxf_tc->drop_frame == 1, mxf_tc->start_frame, mxf) == 0) {
Same as above
+ mxf_add_timecode_metadata(&mxf->fc->metadata, "timecode", &tc);
It looks like you are replacing the timecode metadata, I guess it's desired?
Structural information should only contain zero or one timecode track, so i guess it's not a problem.
BTW, what about the SMPTE timecode in the "system item"?
I think It could be part of an another patch.
Also, you might want to update doc/general.texi if the feature is complete.
New patch attached. Thanks
[...]
-- Clément B.
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
2012/2/23 Matthieu Bouron <matthieu.bouron@gmail.com>:
2012/2/23 Clément Bœsch <ubitux@gmail.com>:
On Thu, Feb 23, 2012 at 06:00:11PM +0100, Matthieu Bouron wrote: [...]
+static int mxf_read_timecode_component(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) +{ + MXFTimecodeComponent *mxf_timecode = arg; + switch(tag) { + case 0x1501: + mxf_timecode->start_frame = avio_rb64(pb); + break; + case 0x1502: + mxf_timecode->rate = (AVRational){avio_rb16(pb), 1}; + break; + case 0x1503: + mxf_timecode->drop_frame = avio_r8(pb); + break; + } + return 0; +} + static int mxf_read_track(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) { MXFTrack *track = arg; @@ -1233,6 +1260,14 @@ finish_decoding_index: return ret; }
+static int mxf_add_timecode_metadata(AVDictionary **pm, const char *key, AVTimecode *tc) +{ + char buf[16];
AV_TIMECODE_STR_SIZE instead of 16
+ av_dict_set(pm, key, av_timecode_make_string(tc, buf, 0), 0); + + return 0; +} + static int mxf_parse_structural_metadata(MXFContext *mxf) { MXFPackage *material_package = NULL; @@ -1257,24 +1292,44 @@ static int mxf_parse_structural_metadata(MXFContext *mxf) MXFTrack *temp_track = NULL; MXFDescriptor *descriptor = NULL; MXFStructuralComponent *component = NULL; + MXFTimecodeComponent *mxf_tc = NULL; UID *essence_container_ul = NULL; const MXFCodecUL *codec_ul = NULL; const MXFCodecUL *container_ul = NULL; AVStream *st; + AVTimecode tc;
if (!(material_track = mxf_resolve_strong_ref(mxf, &material_package->tracks_refs[i], Track))) { av_log(mxf->fc, AV_LOG_ERROR, "could not resolve material track strong ref\n"); continue; }
+ if ((component = mxf_resolve_strong_ref(mxf, &material_track->sequence_ref, TimecodeComponent))) { + mxf_tc = (MXFTimecodeComponent*)component; + if (av_timecode_init(&tc, mxf_tc->rate, mxf_tc->drop_frame == 1, mxf_tc->start_frame, mxf) == 0) {
Better use a variable like flags = mxf_tc->drop_frame == 1 ? AV_TIMECODE_FLAG_DROPFRAME : 0, instead of assuming the value of the flag.
+ mxf_add_timecode_metadata(&mxf->fc->metadata, "timecode", &tc); + } + } + if (!(material_track->sequence = mxf_resolve_strong_ref(mxf, &material_track->sequence_ref, Sequence))) { av_log(mxf->fc, AV_LOG_ERROR, "could not resolve material track sequence strong ref\n"); continue; }
+ for (j = 0; j < material_track->sequence->structural_components_count; j++) { + component = mxf_resolve_strong_ref(mxf, &material_track->sequence->structural_components_refs[j], TimecodeComponent); + if (!component) + continue; + + mxf_tc = (MXFTimecodeComponent*)component; + if (av_timecode_init(&tc, mxf_tc->rate, mxf_tc->drop_frame == 1, mxf_tc->start_frame, mxf) == 0) {
Same as above
+ mxf_add_timecode_metadata(&mxf->fc->metadata, "timecode", &tc);
It looks like you are replacing the timecode metadata, I guess it's desired?
Structural information should only contain zero or one timecode track, so i guess it's not a problem.
BTW, what about the SMPTE timecode in the "system item"?
I think It could be part of an another patch.
Also, you might want to update doc/general.texi if the feature is complete.
New patch attached. Thanks
Ping. Rebased patch attached.
[...]
-- Clément B.
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Fri, Mar 09, 2012 at 06:33:30PM +0100, Matthieu Bouron wrote:
2012/2/23 Matthieu Bouron <matthieu.bouron@gmail.com>:
2012/2/23 Clément Bœsch <ubitux@gmail.com>:
On Thu, Feb 23, 2012 at 06:00:11PM +0100, Matthieu Bouron wrote: [...]
+static int mxf_read_timecode_component(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) +{ + MXFTimecodeComponent *mxf_timecode = arg; + switch(tag) { + case 0x1501: + mxf_timecode->start_frame = avio_rb64(pb); + break; + case 0x1502: + mxf_timecode->rate = (AVRational){avio_rb16(pb), 1}; + break; + case 0x1503: + mxf_timecode->drop_frame = avio_r8(pb); + break; + } + return 0; +} + static int mxf_read_track(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) { MXFTrack *track = arg; @@ -1233,6 +1260,14 @@ finish_decoding_index: return ret; }
+static int mxf_add_timecode_metadata(AVDictionary **pm, const char *key, AVTimecode *tc) +{ + char buf[16];
AV_TIMECODE_STR_SIZE instead of 16
+ av_dict_set(pm, key, av_timecode_make_string(tc, buf, 0), 0); + + return 0; +} + static int mxf_parse_structural_metadata(MXFContext *mxf) { MXFPackage *material_package = NULL; @@ -1257,24 +1292,44 @@ static int mxf_parse_structural_metadata(MXFContext *mxf) MXFTrack *temp_track = NULL; MXFDescriptor *descriptor = NULL; MXFStructuralComponent *component = NULL; + MXFTimecodeComponent *mxf_tc = NULL; UID *essence_container_ul = NULL; const MXFCodecUL *codec_ul = NULL; const MXFCodecUL *container_ul = NULL; AVStream *st; + AVTimecode tc;
if (!(material_track = mxf_resolve_strong_ref(mxf, &material_package->tracks_refs[i], Track))) { av_log(mxf->fc, AV_LOG_ERROR, "could not resolve material track strong ref\n"); continue; }
+ if ((component = mxf_resolve_strong_ref(mxf, &material_track->sequence_ref, TimecodeComponent))) { + mxf_tc = (MXFTimecodeComponent*)component; + if (av_timecode_init(&tc, mxf_tc->rate, mxf_tc->drop_frame == 1, mxf_tc->start_frame, mxf) == 0) {
Better use a variable like flags = mxf_tc->drop_frame == 1 ? AV_TIMECODE_FLAG_DROPFRAME : 0, instead of assuming the value of the flag.
+ mxf_add_timecode_metadata(&mxf->fc->metadata, "timecode", &tc); + } + } + if (!(material_track->sequence = mxf_resolve_strong_ref(mxf, &material_track->sequence_ref, Sequence))) { av_log(mxf->fc, AV_LOG_ERROR, "could not resolve material track sequence strong ref\n"); continue; }
+ for (j = 0; j < material_track->sequence->structural_components_count; j++) { + component = mxf_resolve_strong_ref(mxf, &material_track->sequence->structural_components_refs[j], TimecodeComponent); + if (!component) + continue; + + mxf_tc = (MXFTimecodeComponent*)component; + if (av_timecode_init(&tc, mxf_tc->rate, mxf_tc->drop_frame == 1, mxf_tc->start_frame, mxf) == 0) {
Same as above
+ mxf_add_timecode_metadata(&mxf->fc->metadata, "timecode", &tc);
It looks like you are replacing the timecode metadata, I guess it's desired?
Structural information should only contain zero or one timecode track, so i guess it's not a problem.
BTW, what about the SMPTE timecode in the "system item"?
I think It could be part of an another patch.
Also, you might want to update doc/general.texi if the feature is complete.
New patch attached. Thanks
Ping. Rebased patch attached.
I interpret Tomas "Looks OK overall." from 2 month ago as that he is ok with this and it looks ok to me too thus applied thanks sorry for the long delay [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Avoid a single point of failure, be that a person or equipment.
On 23/02/12 17:00, Matthieu Bouron wrote:
[...]
Patch updated using the new timecode API. I also check if a timecode component is not directly strong referenced by a timecode track (no sequence). It seems to be the case in the BBC "preservation files". Anyway is this not a violation of the spec ?
I think the problem with mxf is the multitudinous ways in which it is interpreted, which is a pain for all concerned. I see the BBC has one format for its "preservation material" that was, I believe, developed by a group headed by the AAF chair, and another for its delivery format as specified by http://www.amwa.tv/ and the AS11 standard with subsets.... I guess having the hooks to pull out whatever is there and then let the user chose is probably a good long term goal, but maybe more than is relevant to this patch.... -- Tim
participants (5)
-
Clément Bœsch -
Matthieu Bouron -
Michael Niedermayer -
Tim Nicholson -
Tomas Härdin