diff mbox series

[FFmpeg-devel,2/2] avformat/wtvdec: Avoid allocations for AVIOContext

Message ID 20200210172138.15596-2-andreas.rheinhardt@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/2] avformat/wtvdec: Forward errors when reading packet
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt Feb. 10, 2020, 5:21 p.m. UTC
The WTV demuxer uses a custom AVIOContext internally (complete with own
seeking and read_packet-functions). And opening one of these entailed
several allocations, including the AVIOContext as well as its opaque.

Yet there is no need to allocate these separately on the heap: For those
AVIOContexts whose lifetime does not extend beyond reading the header
the AVIOContext and the opaque can be put on the stack; whereas the
AVIOContext that is kept (and used to read the actual packets) can be
made a part of the demuxer's context.

By combining the AVIOContext and the opaque into a single structure it
is also more clear which AVIOContext is of the custom type and needs to
be closed manually (via wtvfile_close()).

Furthermore, this commit changes how the AVIOContext is returned: Given
that the destination for the AVIOContext is now prescribed by the caller
and no longer allocated by wtvfile_open_sector(), it is returned via
the same pointer argument and no longer via the ordinary return value;
instead an int is returned which makes it possible to return better error
codes and to distinguish between allocation failures and other errors.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/wtvdec.c | 125 ++++++++++++++++++++-----------------------
 1 file changed, 59 insertions(+), 66 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
index 83f510b92f..880c4a7b49 100644
--- a/libavformat/wtvdec.c
+++ b/libavformat/wtvdec.c
@@ -32,6 +32,7 @@ 
 #include "libavutil/intfloat.h"
 #include "libavutil/time_internal.h"
 #include "avformat.h"
+#include "avio_internal.h"
 #include "internal.h"
 #include "wtv.h"
 #include "mpegts.h"
@@ -59,6 +60,11 @@  typedef struct WtvFile {
     int64_t length;
 } WtvFile;
 
+typedef struct WtvAVIOContext {
+    AVIOContext pb;
+    WtvFile     opaque;
+} WtvAVIOContext;
+
 static int64_t seek_by_sector(AVIOContext *pb, int64_t sector, int64_t offset)
 {
     return avio_seek(pb, (sector << WTV_SECTOR_BITS) + offset, SEEK_SET);
@@ -146,35 +152,32 @@  static int read_ints(AVIOContext *pb, uint32_t *data, int count)
  * @param first_sector  First sector
  * @param length        Length of file (bytes)
  * @param depth         File allocation table depth
- * @return NULL on error
+ * @return <0 on error
  */
-static AVIOContext * wtvfile_open_sector(unsigned first_sector, uint64_t length, int depth, AVFormatContext *s)
+static int wtvfile_open_sector(WtvAVIOContext *pb, AVFormatContext *s,
+                               unsigned first_sector, uint64_t length, int depth)
 {
-    AVIOContext *pb;
     WtvFile *wf;
     uint8_t *buffer;
-    int64_t size;
+    int64_t ret, size;
 
-    if (seek_by_sector(s->pb, first_sector, 0) < 0)
-        return NULL;
+    if ((ret = seek_by_sector(s->pb, first_sector, 0)) < 0)
+        return ret;
 
-    wf = av_mallocz(sizeof(WtvFile));
-    if (!wf)
-        return NULL;
+    wf = &pb->opaque;
+    memset(wf, 0, sizeof(*wf));
 
     if (depth == 0) {
         wf->sectors = av_malloc(sizeof(uint32_t));
         if (!wf->sectors) {
-            av_free(wf);
-            return NULL;
+            return AVERROR(ENOMEM);
         }
         wf->sectors[0]  = first_sector;
         wf->nb_sectors  = 1;
     } else if (depth == 1) {
         wf->sectors = av_malloc(WTV_SECTOR_SIZE);
         if (!wf->sectors) {
-            av_free(wf);
-            return NULL;
+            return AVERROR(ENOMEM);
         }
         wf->nb_sectors  = read_ints(s->pb, wf->sectors, WTV_SECTOR_SIZE / 4);
     } else if (depth == 2) {
@@ -184,8 +187,7 @@  static AVIOContext * wtvfile_open_sector(unsigned first_sector, uint64_t length,
 
         wf->sectors = av_malloc_array(nb_sectors1, 1 << WTV_SECTOR_BITS);
         if (!wf->sectors) {
-            av_free(wf);
-            return NULL;
+            return AVERROR(ENOMEM);
         }
         wf->nb_sectors = 0;
         for (i = 0; i < nb_sectors1; i++) {
@@ -195,15 +197,13 @@  static AVIOContext * wtvfile_open_sector(unsigned first_sector, uint64_t length,
         }
     } else {
         av_log(s, AV_LOG_ERROR, "unsupported file allocation table depth (0x%x)\n", depth);
-        av_free(wf);
-        return NULL;
+        return AVERROR_INVALIDDATA;
     }
     wf->sector_bits = length & (1ULL<<63) ? WTV_SECTOR_BITS : WTV_BIGSECTOR_BITS;
 
     if (!wf->nb_sectors) {
         av_freep(&wf->sectors);
-        av_freep(&wf);
-        return NULL;
+        return AVERROR_INVALIDDATA;
     }
 
     size = avio_size(s->pb);
@@ -220,28 +220,21 @@  static AVIOContext * wtvfile_open_sector(unsigned first_sector, uint64_t length,
 
     /* seek to initial sector */
     wf->position = 0;
-    if (seek_by_sector(s->pb, wf->sectors[0], 0) < 0) {
+    if ((ret = seek_by_sector(s->pb, wf->sectors[0], 0)) < 0) {
         av_freep(&wf->sectors);
-        av_freep(&wf);
-        return NULL;
+        return ret;
     }
 
     wf->pb_filesystem = s->pb;
     buffer = av_malloc(1 << wf->sector_bits);
     if (!buffer) {
         av_freep(&wf->sectors);
-        av_freep(&wf);
-        return NULL;
+        return AVERROR(ENOMEM);
     }
 
-    pb = avio_alloc_context(buffer, 1 << wf->sector_bits, 0, wf,
+    ffio_init_context(&pb->pb, buffer, 1 << wf->sector_bits, 0, wf,
                            wtvfile_read_packet, NULL, wtvfile_seek);
-    if (!pb) {
-        av_freep(&buffer);
-        av_freep(&wf->sectors);
-        av_freep(&wf);
-    }
-    return pb;
+    return 0;
 }
 
 /**
@@ -250,9 +243,10 @@  static AVIOContext * wtvfile_open_sector(unsigned first_sector, uint64_t length,
  * @param      buf_size  directory buffer size
  * @param[in]  filename
  * @param      filename_size size of filename
- * @return NULL on error
+ * @return <0 on error
  */
-static AVIOContext * wtvfile_open2(AVFormatContext *s, const uint8_t *buf, int buf_size, const uint8_t *filename, int filename_size)
+static int wtvfile_open2(WtvAVIOContext *pb, AVFormatContext *s, const uint8_t *buf,
+                         int buf_size, const uint8_t *filename, int filename_size)
 {
     const uint8_t *buf_end = buf + buf_size;
 
@@ -285,26 +279,23 @@  static AVIOContext * wtvfile_open2(AVFormatContext *s, const uint8_t *buf, int b
         if (name_size >= filename_size &&
             !memcmp(name, filename, filename_size) &&
             (name_size < filename_size + 2 || !AV_RN16(name + filename_size)))
-            return wtvfile_open_sector(first_sector, file_length, depth, s);
+            return wtvfile_open_sector(pb, s, first_sector, file_length, depth);
 
         buf += dir_length;
     }
-    return NULL;
+    return AVERROR_INVALIDDATA;
 }
 
-#define wtvfile_open(s, buf, buf_size, filename) \
-    wtvfile_open2(s, buf, buf_size, filename, sizeof(filename))
+#define wtvfile_open(pb, s, buf, buf_size, filename)             \
+    wtvfile_open2(pb, s, buf, buf_size, filename, sizeof(filename))
 
 /**
  * Close file opened with wtvfile_open_sector(), or wtv_open()
  */
-static void wtvfile_close(AVIOContext *pb)
+static void wtvfile_close(WtvAVIOContext *pb)
 {
-    WtvFile *wf = pb->opaque;
-    av_freep(&wf->sectors);
-    av_freep(&pb->opaque);
-    av_freep(&pb->buffer);
-    avio_context_free(&pb);
+    av_freep(&pb->opaque.sectors);
+    av_freep(&pb->pb.buffer);
 }
 
 /*
@@ -316,7 +307,7 @@  typedef struct WtvStream {
 } WtvStream;
 
 typedef struct WtvContext {
-    AVIOContext *pb;       /**< timeline file */
+    WtvAVIOContext pb;       /**< timeline file */
     int64_t epoch;
     int64_t pts;             /**< pts for next data chunk */
     int64_t last_valid_pts;  /**< latest valid pts, used for interactive seeking */
@@ -561,7 +552,7 @@  static void parse_legacy_attrib(AVFormatContext *s, AVIOContext *pb)
 static int parse_videoinfoheader2(AVFormatContext *s, AVStream *st)
 {
     WtvContext *wtv = s->priv_data;
-    AVIOContext *pb = wtv->pb;
+    AVIOContext *pb = &wtv->pb.pb;
 
     avio_skip(pb, 72);  // picture aspect ratio is unreliable
     st->codecpar->codec_tag = ff_get_bmp_header(pb, st, NULL);
@@ -640,7 +631,7 @@  static AVStream * parse_media_type(AVFormatContext *s, AVStream *st, int sid,
                                    ff_asf_guid formattype, uint64_t size)
 {
     WtvContext *wtv = s->priv_data;
-    AVIOContext *pb = wtv->pb;
+    AVIOContext *pb = &wtv->pb.pb;
     if (!ff_guidcmp(subtype, ff_mediasubtype_cpfilters_processed) &&
         !ff_guidcmp(formattype, ff_format_cpfilters_processed)) {
         ff_asf_guid actual_subtype;
@@ -758,7 +749,7 @@  enum {
  */
 static int recover(WtvContext *wtv, uint64_t broken_pos)
 {
-    AVIOContext *pb = wtv->pb;
+    AVIOContext *pb = &wtv->pb.pb;
     int i;
     for (i = 0; i < wtv->nb_index_entries; i++) {
         if (wtv->index_entries[i].pos > broken_pos) {
@@ -782,7 +773,7 @@  static int recover(WtvContext *wtv, uint64_t broken_pos)
 static int parse_chunks(AVFormatContext *s, int mode, int64_t seekts, int *len_ptr)
 {
     WtvContext *wtv = s->priv_data;
-    AVIOContext *pb = wtv->pb;
+    AVIOContext *pb = &wtv->pb.pb;
     while (!avio_feof(pb)) {
         ff_asf_guid g;
         int len, sid, consumed;
@@ -959,7 +950,8 @@  static int read_header(AVFormatContext *s)
     unsigned root_sector;
     int root_size;
     uint8_t root[WTV_SECTOR_SIZE];
-    AVIOContext *pb;
+    WtvAVIOContext tmp;
+    AVIOContext *pb = &tmp.pb;
     int64_t timeline_pos;
     int64_t ret;
 
@@ -985,26 +977,27 @@  static int read_header(AVFormatContext *s)
         return AVERROR_INVALIDDATA;
 
     /* parse chunks up until first data chunk */
-    wtv->pb = wtvfile_open(s, root, root_size, ff_timeline_le16);
-    if (!wtv->pb) {
+    ret = wtvfile_open(&wtv->pb, s, root, root_size, ff_timeline_le16);
+    if (ret < 0) {
         av_log(s, AV_LOG_ERROR, "timeline data missing\n");
-        return AVERROR_INVALIDDATA;
+        return ret;
     }
 
     ret = parse_chunks(s, SEEK_TO_DATA, 0, 0);
     if (ret < 0) {
-        wtvfile_close(wtv->pb);
+        wtvfile_close(&wtv->pb);
         return ret;
     }
-    avio_seek(wtv->pb, -32, SEEK_CUR);
+    avio_seek(&wtv->pb.pb, -32, SEEK_CUR);
 
     timeline_pos = avio_tell(s->pb); // save before opening another file
 
     /* read metadata */
-    pb = wtvfile_open(s, root, root_size, ff_table_0_entries_legacy_attrib_le16);
-    if (pb) {
+    ret = wtvfile_open(&tmp, s, root, root_size,
+                       ff_table_0_entries_legacy_attrib_le16);
+    if (ret >= 0) {
         parse_legacy_attrib(s, pb);
-        wtvfile_close(pb);
+        wtvfile_close(&tmp);
     }
 
     s->ctx_flags |= AVFMTCTX_NOHEADER; // Needed for noStreams.wtv
@@ -1012,8 +1005,8 @@  static int read_header(AVFormatContext *s)
     /* read seek index */
     if (s->nb_streams) {
         AVStream *st = s->streams[0];
-        pb = wtvfile_open(s, root, root_size, ff_table_0_entries_time_le16);
-        if (pb) {
+        ret = wtvfile_open(&tmp, s, root, root_size, ff_table_0_entries_time_le16);
+        if (ret >= 0) {
             while(1) {
                 uint64_t timestamp = avio_rl64(pb);
                 uint64_t frame_nb  = avio_rl64(pb);
@@ -1022,11 +1015,11 @@  static int read_header(AVFormatContext *s)
                 ff_add_index_entry(&wtv->index_entries, &wtv->nb_index_entries, &wtv->index_entries_allocated_size,
                                    0, timestamp, frame_nb, 0, AVINDEX_KEYFRAME);
             }
-            wtvfile_close(pb);
+            wtvfile_close(&tmp);
 
             if (wtv->nb_index_entries) {
-                pb = wtvfile_open(s, root, root_size, ff_timeline_table_0_entries_Events_le16);
-                if (pb) {
+                ret = wtvfile_open(&tmp, s, root, root_size, ff_timeline_table_0_entries_Events_le16);
+                if (ret >= 0) {
                     AVIndexEntry *e = wtv->index_entries;
                     AVIndexEntry *e_end = wtv->index_entries + wtv->nb_index_entries - 1;
                     uint64_t last_position = 0;
@@ -1042,7 +1035,7 @@  static int read_header(AVFormatContext *s)
                         last_position = position;
                     }
                     e_end->pos = last_position;
-                    wtvfile_close(pb);
+                    wtvfile_close(&tmp);
                     st->duration = e_end->timestamp;
                 }
             }
@@ -1056,7 +1049,7 @@  static int read_header(AVFormatContext *s)
 static int read_packet(AVFormatContext *s, AVPacket *pkt)
 {
     WtvContext *wtv = s->priv_data;
-    AVIOContext *pb = wtv->pb;
+    AVIOContext *pb = &wtv->pb.pb;
     int stream_index, len, ret;
 
     stream_index = parse_chunks(s, SEEK_TO_DATA, 0, &len);
@@ -1076,7 +1069,7 @@  static int read_seek(AVFormatContext *s, int stream_index,
                      int64_t ts, int flags)
 {
     WtvContext *wtv = s->priv_data;
-    AVIOContext *pb = wtv->pb;
+    AVIOContext *pb = &wtv->pb.pb;
     AVStream *st = s->streams[0];
     int64_t ts_relative;
     int i;
@@ -1116,7 +1109,7 @@  static int read_close(AVFormatContext *s)
 {
     WtvContext *wtv = s->priv_data;
     av_freep(&wtv->index_entries);
-    wtvfile_close(wtv->pb);
+    wtvfile_close(&wtv->pb);
     return 0;
 }