[FFmpeg-devel] [PATCH] mxfdec: do not fail on error parsing footer.

[Tomas Härdin]

On Thu, 2012-01-05 at 22:06 +0100, Reimar Döffinger wrote:

Oops, didn't notice this thread until Michael pointed it out to me.

> Failing it play a file just because some metadata was not parseable
> is overkill.
> This should fix trac issue #879.

Uhm, 10bit_yuv_j2k.mxf plays fine with current master and
--enable-libopenjpeg.
It seems we can't fix the ticket properly without the whole sample file.
I'd ask of the reporter to upload it somewhere.

> Signed-off-by: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
> ---
>  libavformat/mxfdec.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index 10512ef..66af2e4 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -1634,7 +1634,7 @@ static int mxf_read_header(AVFormatContext *s, AVFormatParameters *ap)
>  
>              if (!mxf->current_partition) {
>                  av_log(mxf->fc, AV_LOG_ERROR, "found essence prior to first PartitionPack\n");
> -                return AVERROR_INVALIDDATA;
> +                goto handle_essence;

#879 doesn't list this error. Files that trigger this violate the spec
hard. Also, mxf_parse_structural_metadata() would create zero streams
due to no metadata, so definately not OK.

>              }
>  
>              if (!mxf->current_partition->essence_offset) {
> @@ -1688,14 +1688,14 @@ static int mxf_read_header(AVFormatContext *s, AVFormatParameters *ap)
>                      if (avio_tell(s->pb) > next) {
>                          av_log(s, AV_LOG_ERROR, "read past end of KLV @ %#"PRIx64"\n",
>                                 klv.offset);
> -                        return AVERROR_INVALIDDATA;
> +                        goto handle_essence;

Not relevant to the ticket AFAICT. I'd rather be more stringent with
what the demuxer accepts, but I guess trying to make sense of the file
anyway doesn't hurt..

>                      }
>  
>                      avio_seek(s->pb, next, SEEK_SET);
>                  }
>                  if (res < 0) {
>                      av_log(s, AV_LOG_ERROR, "error reading header metadata\n");
> -                    return -1;
> +                    goto handle_essence;

This is the wrong place to fix #879 - my guess is
mxf_read_partition_pack() needs to be a bit more clever. Might be OK on
its own though, just like the previous hunk.

/Tomas