Hi,

the attached patch seems to fix ticket #3773, but streamcopy is such a
core feature of ffmpeg that I assume I'm missing a use case or
incorrectly handle the buffers referencing.

Another note is that this patch was developed without a memory tracer
like valgrind: I have just validated that the test case doesn't crash
and passes fate-h264-bsf-mp4toannexb.

Last point, I think this bug raises a few questions/issues:
- API may not be abundantly clear that AVPacket::data is refcounted
and should not be meddled with directly;
- For lack of a way to do it, ffmpeg.c streamcopy meddles with AVPacket::data;
- AVPacket::side_data is not ref-counted; because of such things as
av_format_inject_global_side_data, it could make sense to have it
separate from AVPacket::data ref-counting.

That last point is probably a major, API-breaking, undertaking. I have
no plan to start on it, in particular because I feel unqualified for
it.

Best regards,
-- 
Christophe
From 7848e2eed0780d5d2b3190b90174d580257a7ec9 Mon Sep 17 00:00:00 2001
From: Christophe Gisquet <christophe.gisq...@gmail.com>
Date: Wed, 13 Aug 2014 18:40:17 +0200
Subject: [PATCH] ffmpeg: fix streamcopy with side data

The issue is that, when the main packet data buffer is changed, streamcopy
uses a temporary new packet to store that buffer, frees the old packet, and
replace it with the new packet.

However, in doing so, it forgets about the side data, which gets freed, but
is still needed and referenced. Then, when the packet gets freed again in
the normal code path, it attempts to free its side data which has already
been freed.

Therefore, simply avoid the first free on side data by removing that side
data from the packet.

Fixes ticket #3773.
---
 ffmpeg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ffmpeg.c b/ffmpeg.c
index b82d2be..60b10cc 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -627,6 +627,8 @@ static void write_frame(AVFormatContext *s, AVPacket *pkt, OutputStream *ost)
                 a = AVERROR(ENOMEM);
         }
         if (a > 0) {
+            pkt->side_data = NULL;
+            pkt->side_data_elems = 0;
             av_free_packet(pkt);
             new_pkt.buf = av_buffer_create(new_pkt.data, new_pkt.size,
                                            av_buffer_default_free, NULL, 0);
-- 
1.9.2.msysgit.0

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

Reply via email to