[FFmpeg-devel] [PATCH] mxfdec: add timecode to metadata

[Matthieu Bouron]

2012/1/16 Tomas Härdin <tomas.hardin at 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 at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel