On Tue, 31 Mar 2020, Andreas Rheinhardt wrote:
The documentation of av_write_frame() explicitly states that the function
doesn't 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.rheinha...@gmail.com>
---
Whether a commit like this is necessary depends upon the meaning of
"ownership" (whether not having ownership disallows one from modifying a
packet) and of the user's expectations (namely if the av_write_frame()
should attempt to not modify the user-provided packet even if it were
allowed to). Furthermore, this commit has the downside of adding an
AVPacket on the stack.
libavformat/mux.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/libavformat/mux.c b/libavformat/mux.c
index 79731d3008..be7652c97b 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -880,11 +880,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);
@@ -895,22 +896,43 @@ 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
- return write_packet(s, pkt);
+ ret = write_packet(s, pkt);
+
+fail:
+ av_packet_unref(pkt);
+ return ret;
}
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".
