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

[Tomas Härdin]

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