@@ -872,11 +872,12 @@ static int do_packet_auto_bsf(AVFormatContext *s, AVPacket *pkt) {
return 1;
}
-int av_write_frame(AVFormatContext *s, AVPacket *pkt)
+int av_write_frame(AVFormatContext *s, AVPacket *in)
{
+ AVPacket local_pkt, *pkt = &local_pkt;
int ret;
- if (!pkt) {
+ if (!in) {
if (s->oformat->flags & AVFMT_ALLOW_FLUSH) {
ret = s->oformat->write_packet(s, NULL);
flush_if_needed(s);
@@ -887,19 +888,36 @@ int av_write_frame(AVFormatContext *s, AVPacket *pkt)
return 1;
}
+ /* We don't own in, so we have to make sure not to modify it.
+ * The following avoids copying in's data unnecessarily.
+ * Copying side data is unavoidable as a bitstream filter
+ * may change it, e.g. free it on errors. */
+ pkt->data = in->data;
+ pkt->size = in->size;
+ if (in->buf) {
+ pkt->buf = av_buffer_ref(in->buf);
+ if (!pkt->buf)
+ return AVERROR(ENOMEM);
+ } else {
+ pkt->buf = NULL;
+ }
+ ret = av_packet_copy_props(pkt, in);
+ if (ret < 0)
+ goto fail;
+
ret = prepare_input_packet(s, pkt);
if (ret < 0)
- return ret;
+ goto fail;
ret = do_packet_auto_bsf(s, pkt);
if (ret <= 0)
- return ret;
+ goto fail;
#if FF_API_COMPUTE_PKT_FIELDS2 && FF_API_LAVF_AVCTX
ret = compute_muxer_pkt_fields(s, s->streams[pkt->stream_index], pkt);
if (ret < 0 && !(s->oformat->flags & AVFMT_NOTIMESTAMPS))
- return ret;
+ goto fail;
#endif
ret = write_packet(s, pkt);
@@ -908,6 +926,9 @@ int av_write_frame(AVFormatContext *s, AVPacket *pkt)
if (ret >= 0)
s->streams[pkt->stream_index]->nb_frames++;
+
+fail:
+ av_packet_unref(pkt);
return ret;
}
The documentation of av_write_frame explicitly states that the function does not take ownership of the packets sent to it; while av_write_frame does not directly unreference the packets after having written them, it nevertheless modifies the packet in various ways: 1. The timestamps might be modified either by prepare_input_packet or compute_muxer_pkt_fields. 2. If a bitstream filter gets applied, it takes ownership of the reference and the side-data in the packet sent to it in av_bsf_send_packet. In case of do_packet_auto_bsf, the end result is that the returned packet contains the output of the last bsf in the chain. If an error happens, an empty packet will be returned; a packet may also simply not lead to any output (vp9_superframe). This also implies that side data needs to be really copied and can't be shared with the input packet. The method choosen here minimizes copying of data: When the input isn't refcounted and no bitstream filter is applied, the packet's data will not be copied. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- I am not sure about this one. If one understands ownership of a packet as "the right to modify a packet as well as the obligation to ultimately free the data" (in case the data is shared, one of course needs to make it writable before modifying it), then it is obvious that av_write_frame needs to preserve the original packet given to it; that was my understanding when writing this patch. But Hendrik has said that "ownership" actually only comes with the obligation to free the data and that av_write_frame does not taking ownership of the packet does not imply that the caller may expect the packet to be returned unmodified. If the latter is correct, then this patch is moot, of course. libavformat/mux.c | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-)