diff mbox series

[FFmpeg-devel,v2] lavf/movenc: Use a dynamic buffer when writing the mfra box

Message ID 20200626125802.526306-1-derek.buitenhuis@gmail.com
State Accepted
Commit d5247fb1da615a7ccff9fa7271bc8aba23e0de25
Headers show
Series [FFmpeg-devel,v2] lavf/movenc: Use a dynamic buffer when writing the mfra box | expand

Checks

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

Commit Message

Derek Buitenhuis June 26, 2020, 12:58 p.m. UTC 
When doing streamed output, with e.g. +dash, if the mfra box ended
up being larger than the AVIOContext write buffer, the (unchecked)
seeking back to update the box size would silently fail and produce
an invalid mfra box.

This is similar to how other boxes are written in fragmented mode.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
It is still weird to me that we prefer all of our internal code
to use ffio_free_dyn_buf but don't allow actual public API users
to use this functionality.
---
 libavformat/movenc.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

Comments

Derek Buitenhuis June 28, 2020, 12:25 p.m. UTC | #1 
On 26/06/2020 13:58, Derek Buitenhuis wrote:
> When doing streamed output, with e.g. +dash, if the mfra box ended
> up being larger than the AVIOContext write buffer, the (unchecked)
> seeking back to update the box size would silently fail and produce
> an invalid mfra box.
> 
> This is similar to how other boxes are written in fragmented mode.
> 
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
> It is still weird to me that we prefer all of our internal code
> to use ffio_free_dyn_buf but don't allow actual public API users
> to use this functionality.
> ---
>  libavformat/movenc.c | 38 ++++++++++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 12 deletions(-)

Will push tonight barring any objections.

- Derek
Derek Buitenhuis June 28, 2020, 7:24 p.m. UTC | #2 
On 28/06/2020 13:25, Derek Buitenhuis wrote:
> Will push tonight barring any objections.

Pushed.

- Derek
diff mbox series

Patch

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index b8e45760ee..7db2e28840 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -4798,28 +4798,40 @@  static int mov_write_tfra_tag(AVIOContext *pb, MOVTrack *track)
 
 static int mov_write_mfra_tag(AVIOContext *pb, MOVMuxContext *mov)
 {
-    int64_t pos = avio_tell(pb);
-    int i;
+    AVIOContext *mfra_pb;
+    int i, ret, sz;
+    uint8_t *buf;
 
-    avio_wb32(pb, 0); /* size placeholder */
-    ffio_wfourcc(pb, "mfra");
+    ret = avio_open_dyn_buf(&mfra_pb);
+    if (ret < 0)
+        return ret;
+
+    avio_wb32(mfra_pb, 0); /* size placeholder */
+    ffio_wfourcc(mfra_pb, "mfra");
     /* An empty mfra atom is enough to indicate to the publishing point that
      * the stream has ended. */
     if (mov->flags & FF_MOV_FLAG_ISML)
-        return update_size(pb, pos);
+        goto done_mfra;
 
     for (i = 0; i < mov->nb_streams; i++) {
         MOVTrack *track = &mov->tracks[i];
         if (track->nb_frag_info)
-            mov_write_tfra_tag(pb, track);
+            mov_write_tfra_tag(mfra_pb, track);
     }
 
-    avio_wb32(pb, 16);
-    ffio_wfourcc(pb, "mfro");
-    avio_wb32(pb, 0); /* version + flags */
-    avio_wb32(pb, avio_tell(pb) + 4 - pos);
+    avio_wb32(mfra_pb, 16);
+    ffio_wfourcc(mfra_pb, "mfro");
+    avio_wb32(mfra_pb, 0); /* version + flags */
+    avio_wb32(mfra_pb, avio_tell(mfra_pb) + 4);
 
-    return update_size(pb, pos);
+done_mfra:
+
+    sz  = update_size(mfra_pb, 0);
+    ret = avio_get_dyn_buf(mfra_pb, &buf);
+    avio_write(pb, buf, ret);
+    ffio_free_dyn_buf(&mfra_pb);
+
+    return sz;
 }
 
 static int mov_write_mdat_tag(AVIOContext *pb, MOVMuxContext *mov)
@@ -6987,7 +6999,9 @@  static int mov_write_trailer(AVFormatContext *s)
         }
         if (!(mov->flags & FF_MOV_FLAG_SKIP_TRAILER)) {
             avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_TRAILER);
-            mov_write_mfra_tag(pb, mov);
+            res = mov_write_mfra_tag(pb, mov);
+            if (res < 0)
+                return res;
         }
     }