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".