[FFmpeg-devel] [PATCH] mxfdec.c: Try TC from Footer if Header TC was < 1

[Tomas Härdin]

mån 2021-05-24 klockan 12:30 +0200 skrev emcodem:
> Added support for reading Start Timecode from Footer (if any). 
> Specifically targets Omneon 6.4.3.0 but also works on other Versions and Vendors, e.g. when Header is OpenIncomplete.
> Function mxf_resolve_strong_ref_reverse can potentially be re-used for getting other values like Duration and 
> Origin from Footer.

This needs a sample and a test

> ---
>  libavformat/mxfdec.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index 3bf480a3a6..557e01f8ed 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -1396,6 +1396,19 @@ static const MXFCodecUL *mxf_get_codec_ul(const MXFCodecUL *uls, UID *uid)
>      return uls;
>  }
>  
> +static void *mxf_resolve_strong_ref_reverse(MXFContext *mxf, UID *strong_ref, enum MXFMetadataSetType type)

This and mxf_resolve_strong_ref() could maybe be merged to one function
with an "int dir" option, and mxf_resolve_strong_ref() just calling it
with the value 1.

> +{
> +    int i;
> +    if (!strong_ref)
> +        return NULL;
> +    for (i = mxf->metadata_sets_count-1; i >= 0 ; i--) {
> +        if (!memcmp(*strong_ref, mxf->metadata_sets[i]->uid, 16) &&
> +            (type == AnyType || mxf->metadata_sets[i]->type == type)) {
> +            return mxf->metadata_sets[i];
> +        }
> +    }
> +    return NULL;
> +}

Missing newline, but I can add that locally

>  static void *mxf_resolve_strong_ref(MXFContext *mxf, UID *strong_ref, enum MXFMetadataSetType type)
>  {
>      int i;
> @@ -2328,8 +2341,15 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
>                  continue;
>  
>              mxf_tc = (MXFTimecodeComponent*)component;
> +            if (mxf_tc->start_frame <= 0) {

I feel like this should trigger on OpenIncomplete instead. I wouldn't
be surprised if start_frame < 0 is perfectly valid.


> +            if (mxf_tc->start_frame <= 0) {
> +                    av_log(mxf->fc, AV_LOG_TRACE, "Header Start Timecode was %d, trying reversed parsing\n",mxf_tc->start_frame);
> +                    component = mxf_resolve_strong_ref_reverse(mxf, &material_track->sequence->structural_components_refs[j], TimecodeComponent);
> +                    mxf_tc = (MXFTimecodeComponent*)component;

Wrong indent. I was also going to comment that mxf_tc might end up NULL
here but it can't since it's non-NULL when resolving in the forward
direction.

/Tomas