[FFmpeg-devel] [PATCH v3 7/8] avformat/utils: Remove unnecessary packet copies
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Tue Sep 10 01:50:02 EEST 2019
Up until now, read_frame_internal in avformat/utils.c uses a spare
packet on the stack that serves no real purpose: At no point in this
function is there a need for another packet besides the packet destined
for output:
1. If the packet doesn't need a parser, but is output as is, the content
of the spare packet (that at this point contains a freshly read packet)
is simply copied into the output packet (via simple assignment, not
av_packet_move_ref, thereby confusing ownership).
2. If the packet needs parsing, the spare packet will be reset after
parsing and any packets resulting from the packet read will be put into
a packet list; the output packet is not used here at all.
3. If the stream should be discarded, the spare packet will be
unreferenced; the output packet is not used here at all.
Therefore the spare packet and the copies can be removed in principle.
In practice, one more thing needs to be taken care of: If ff_read_packet
failed, this did not affect the output packet, now it does. This
potential problem is solved by making sure that ff_read_packet always
resets the output packet in case of errors.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
Resending to fix a typo mentioned by Andriy. It's also intended to ping
the whole patchset.
libavformat/utils.c | 51 ++++++++++++++++++++++-----------------------
1 file changed, 25 insertions(+), 26 deletions(-)
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 66b33df165..5f558d8791 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -830,6 +830,10 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
int ret, i, err;
AVStream *st;
+ pkt->data = NULL;
+ pkt->size = 0;
+ av_init_packet(pkt);
+
for (;;) {
AVPacketList *pktl = s->internal->raw_packet_buffer;
const AVPacket *pkt1;
@@ -847,11 +851,11 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
}
}
- pkt->data = NULL;
- pkt->size = 0;
- av_init_packet(pkt);
ret = s->iformat->read_packet(s, pkt);
if (ret < 0) {
+ pkt->data = NULL;
+ pkt->size = 0;
+ av_init_packet(pkt);
/* Some demuxers return FFERROR_REDO when they consume
data and discard it (ignored streams, junk, extradata).
We must re-call the demuxer to get the real packet. */
@@ -1579,10 +1583,9 @@ static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
while (!got_packet && !s->internal->parse_queue) {
AVStream *st;
- AVPacket cur_pkt;
/* read next packet */
- ret = ff_read_packet(s, &cur_pkt);
+ ret = ff_read_packet(s, pkt);
if (ret < 0) {
if (ret == AVERROR(EAGAIN))
return ret;
@@ -1597,7 +1600,7 @@ static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
break;
}
ret = 0;
- st = s->streams[cur_pkt.stream_index];
+ st = s->streams[pkt->stream_index];
/* update context if required */
if (st->internal->need_context_update) {
@@ -1615,7 +1618,7 @@ static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
ret = avcodec_parameters_to_context(st->internal->avctx, st->codecpar);
if (ret < 0) {
- av_packet_unref(&cur_pkt);
+ av_packet_unref(pkt);
return ret;
}
@@ -1624,7 +1627,7 @@ FF_DISABLE_DEPRECATION_WARNINGS
/* update deprecated public codec context */
ret = avcodec_parameters_to_context(st->codec, st->codecpar);
if (ret < 0) {
- av_packet_unref(&cur_pkt);
+ av_packet_unref(pkt);
return ret;
}
FF_ENABLE_DEPRECATION_WARNINGS
@@ -1633,23 +1636,23 @@ FF_ENABLE_DEPRECATION_WARNINGS
st->internal->need_context_update = 0;
}
- if (cur_pkt.pts != AV_NOPTS_VALUE &&
- cur_pkt.dts != AV_NOPTS_VALUE &&
- cur_pkt.pts < cur_pkt.dts) {
+ if (pkt->pts != AV_NOPTS_VALUE &&
+ pkt->dts != AV_NOPTS_VALUE &&
+ pkt->pts < pkt->dts) {
av_log(s, AV_LOG_WARNING,
"Invalid timestamps stream=%d, pts=%s, dts=%s, size=%d\n",
- cur_pkt.stream_index,
- av_ts2str(cur_pkt.pts),
- av_ts2str(cur_pkt.dts),
- cur_pkt.size);
+ pkt->stream_index,
+ av_ts2str(pkt->pts),
+ av_ts2str(pkt->dts),
+ pkt->size);
}
if (s->debug & FF_FDEBUG_TS)
av_log(s, AV_LOG_DEBUG,
"ff_read_packet stream=%d, pts=%s, dts=%s, size=%d, duration=%"PRId64", flags=%d\n",
- cur_pkt.stream_index,
- av_ts2str(cur_pkt.pts),
- av_ts2str(cur_pkt.dts),
- cur_pkt.size, cur_pkt.duration, cur_pkt.flags);
+ pkt->stream_index,
+ av_ts2str(pkt->pts),
+ av_ts2str(pkt->dts),
+ pkt->size, pkt->duration, pkt->flags);
if (st->need_parsing && !st->parser && !(s->flags & AVFMT_FLAG_NOPARSE)) {
st->parser = av_parser_init(st->codecpar->codec_id);
@@ -1669,7 +1672,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
if (!st->need_parsing || !st->parser) {
/* no parsing needed: we just output the packet as is */
- *pkt = cur_pkt;
compute_pkt_fields(s, st, NULL, pkt, AV_NOPTS_VALUE, AV_NOPTS_VALUE);
if ((s->iformat->flags & AVFMT_GENERIC_INDEX) &&
(pkt->flags & AV_PKT_FLAG_KEY) && pkt->dts != AV_NOPTS_VALUE) {
@@ -1679,7 +1681,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
}
got_packet = 1;
} else if (st->discard < AVDISCARD_ALL) {
- if ((ret = parse_packet(s, &cur_pkt, cur_pkt.stream_index)) < 0)
+ if ((ret = parse_packet(s, pkt, pkt->stream_index)) < 0)
return ret;
st->codecpar->sample_rate = st->internal->avctx->sample_rate;
st->codecpar->bit_rate = st->internal->avctx->bit_rate;
@@ -1688,15 +1690,12 @@ FF_ENABLE_DEPRECATION_WARNINGS
st->codecpar->codec_id = st->internal->avctx->codec_id;
} else {
/* free packet */
- av_packet_unref(&cur_pkt);
+ av_packet_unref(pkt);
}
if (pkt->flags & AV_PKT_FLAG_KEY)
st->skip_to_keyframe = 0;
if (st->skip_to_keyframe) {
- av_packet_unref(&cur_pkt);
- if (got_packet) {
- *pkt = cur_pkt;
- }
+ av_packet_unref(pkt);
got_packet = 0;
}
}
--
2.21.0
More information about the ffmpeg-devel
mailing list