diff mbox series

[FFmpeg-devel,3/9] avformat/mxfdec: Simplify cleanup after read_header failure

Message ID 20200721021215.32647-1-andreas.rheinhardt@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v2,1/2] avformat: Redo cleanup of demuxer upon read_header() failure | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt July 21, 2020, 2:12 a.m. UTC 
by setting the AVFMT_HEADER_CLEANUP flag.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/mxfdec.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

Comments

Tomas Härdin July 22, 2020, 12:45 p.m. UTC | #1 
tis 2020-07-21 klockan 04:12 +0200 skrev Andreas Rheinhardt:
> by setting the AVFMT_HEADER_CLEANUP flag.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/mxfdec.c | 24 +++++++-----------------
>  1 file changed, 7 insertions(+), 17 deletions(-)

I was confused for a while since "git grep AVFMT_HEADER_CLEANUP" didn't
give any results, before I noticed this is part of a patchset.

This makes cleanup much cleaner -> approved!

/Tomas
diff mbox series

Patch

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 90546d42b3..06c6e0890b 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -302,8 +302,6 @@  typedef struct MXFMetadataReadTableEntry {
     enum MXFMetadataSetType type;
 } MXFMetadataReadTableEntry;
 
-static int mxf_read_close(AVFormatContext *s);
-
 /* partial keys to match */
 static const uint8_t mxf_header_partition_pack_key[]       = { 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02 };
 static const uint8_t mxf_essence_element_key[]             = { 0x06,0x0e,0x2b,0x34,0x01,0x02,0x01,0x01,0x0d,0x01,0x03,0x01 };
@@ -3169,7 +3167,6 @@  static int mxf_read_header(AVFormatContext *s)
 
     if (!mxf_read_sync(s->pb, mxf_header_partition_pack_key, 14)) {
         av_log(s, AV_LOG_ERROR, "could not find header partition pack key\n");
-        //goto fail should not be needed as no metadata sets will have been parsed yet
         return AVERROR_INVALIDDATA;
     }
     avio_seek(s->pb, -14, SEEK_CUR);
@@ -3200,8 +3197,7 @@  static int mxf_read_header(AVFormatContext *s)
 
             if (!mxf->current_partition) {
                 av_log(mxf->fc, AV_LOG_ERROR, "found essence prior to first PartitionPack\n");
-                ret = AVERROR_INVALIDDATA;
-                goto fail;
+                return AVERROR_INVALIDDATA;
             }
 
             if (!mxf->current_partition->first_essence_klv.offset)
@@ -3226,7 +3222,7 @@  static int mxf_read_header(AVFormatContext *s)
         for (metadata = mxf_metadata_read_table; metadata->read; metadata++) {
             if (IS_KLV_KEY(klv.key, metadata->key)) {
                 if ((ret = mxf_parse_klv(mxf, klv, metadata->read, metadata->ctx_size, metadata->type)) < 0)
-                    goto fail;
+                    return ret;
                 break;
             }
         }
@@ -3239,21 +3235,20 @@  static int mxf_read_header(AVFormatContext *s)
     /* FIXME avoid seek */
     if (!essence_offset)  {
         av_log(s, AV_LOG_ERROR, "no essence\n");
-        ret = AVERROR_INVALIDDATA;
-        goto fail;
+        return AVERROR_INVALIDDATA;
     }
     avio_seek(s->pb, essence_offset, SEEK_SET);
 
     /* we need to do this before computing the index tables
      * to be able to fill in zero IndexDurations with st->duration */
     if ((ret = mxf_parse_structural_metadata(mxf)) < 0)
-        goto fail;
+        return ret;
 
     for (int i = 0; i < s->nb_streams; i++)
         mxf_handle_missing_index_segment(mxf, s->streams[i]);
 
     if ((ret = mxf_compute_index_tables(mxf)) < 0)
-        goto fail;
+        return ret;
 
     if (mxf->nb_index_tables > 1) {
         /* TODO: look up which IndexSID to use via EssenceContainerData */
@@ -3261,8 +3256,7 @@  static int mxf_read_header(AVFormatContext *s)
                mxf->nb_index_tables, mxf->index_tables[0].index_sid);
     } else if (mxf->nb_index_tables == 0 && mxf->op == OPAtom && (s->error_recognition & AV_EF_EXPLODE)) {
         av_log(mxf->fc, AV_LOG_ERROR, "cannot demux OPAtom without an index\n");
-        ret = AVERROR_INVALIDDATA;
-        goto fail;
+        return AVERROR_INVALIDDATA;
     }
 
     mxf_compute_essence_containers(s);
@@ -3271,10 +3265,6 @@  static int mxf_read_header(AVFormatContext *s)
         mxf_compute_edit_units_per_packet(mxf, s->streams[i]);
 
     return 0;
-fail:
-    mxf_read_close(s);
-
-    return ret;
 }
 
 /* Get the edit unit of the next packet from current_offset in a track. The returned edit unit can be original_duration as well! */
@@ -3740,7 +3730,7 @@  static const AVClass demuxer_class = {
 AVInputFormat ff_mxf_demuxer = {
     .name           = "mxf",
     .long_name      = NULL_IF_CONFIG_SMALL("MXF (Material eXchange Format)"),
-    .flags          = AVFMT_SEEK_TO_PTS,
+    .flags          = AVFMT_SEEK_TO_PTS | AVFMT_HEADER_CLEANUP,
     .priv_data_size = sizeof(MXFContext),
     .read_probe     = mxf_probe,
     .read_header    = mxf_read_header,