On Tue, 31 Mar 2020, Andreas Rheinhardt wrote:

If writing an uncoded frame fails at the preparatory steps of
av_[interleaved_]write_frame(), the frame would be either not freed
at all in case of av_write_frame() or would leak when the fake packet
would be unreferenced like an ordinary packet.
There is also a memleak when the output format is not suited for
writing uncoded frames as the documentation states that ownership of the
frame passes to av_[interleaved_]write_uncoded_frame(). Both of these
memleaks have been fixed.

Yuck, does using uncoded_frames muxing have any benefit over using AV_CODEC_ID_WRAPPED_AVFRAME with normal muxing? If not, then I'd very much like to see uncoded frames dropped with the next bump...


Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
---
libavformat/mux.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/libavformat/mux.c b/libavformat/mux.c
index 814d773b9d..a0ebcec119 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -1239,7 +1239,11 @@ int av_interleaved_write_frame(AVFormatContext *s, 
AVPacket *pkt)
            return ret;
    }
fail:
-    av_packet_unref(pkt);
+    // This is a deviation from the usual behaviour
+    // of av_interleaved_write_frame: We leave cleaning
+    // up uncoded frames to write_uncoded_frame_internal.
+    if (!(pkt->flags & AV_PKT_FLAG_UNCODED_FRAME))
+        av_packet_unref(pkt);
    return ret;
}

@@ -1324,10 +1328,13 @@ static int write_uncoded_frame_internal(AVFormatContext 
*s, int stream_index,
                                        AVFrame *frame, int interleaved)
{
    AVPacket pkt, *pktp;
+    int ret;

    av_assert0(s->oformat);
-    if (!s->oformat->write_uncoded_frame)
-        return AVERROR(ENOSYS);
+    if (!s->oformat->write_uncoded_frame) {
+        ret = AVERROR(ENOSYS);
+        goto free;
+    }

    if (!frame) {
        pktp = NULL;
@@ -1343,8 +1350,14 @@ static int write_uncoded_frame_internal(AVFormatContext 
*s, int stream_index,
        pkt.flags |= AV_PKT_FLAG_UNCODED_FRAME;
    }

-    return interleaved ? av_interleaved_write_frame(s, pktp) :
-                         av_write_frame(s, pktp);
+    ret = interleaved ? av_interleaved_write_frame(s, pktp) :
+                        av_write_frame(s, pktp);
+    if (ret < 0 && pktp) {
+        frame = (AVFrame*)pktp->data;
+    free:

I think we always align labels to the start of the line.

+        av_frame_free(&frame);
+    }
+    return ret;
}

int av_write_uncoded_frame(AVFormatContext *s, int stream_index,

LGTM, thanks.

Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to