From patchwork Mon Sep 9 22:50:02 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 14991 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 722DE44A736 for ; Tue, 10 Sep 2019 01:57:47 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5084A680508; Tue, 10 Sep 2019 01:57:47 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id BC9D6680508 for ; Tue, 10 Sep 2019 01:57:40 +0300 (EEST) Received: by mail-wr1-f65.google.com with SMTP id h7so15286683wrw.8 for ; Mon, 09 Sep 2019 15:57:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=j/5Jn7Rovm3p9y4iOyPLRnFhbvc8dPPQk+nWn+j3koc=; b=m9p7GCT9T1M3EBgaWBpeCGJjbir1C8TPy9A0Gbf/lwT/3/yEocigwYTsD8b0WTJaIC RIrB09YcK8Bic+M1H/RlNoVItpHs7c0gLmtrlYUaSSqbzTj7nqkUJk+AXxP7np30SPlE g9ZcNwhssdStsD2hcHTbhQuObDEGkNOs8xZBrAZKbGOnTuhoJ2WKfCBu5pp/QxrVZjpo Ha9VIKaoCAuxaxncXgoU9jz+5R0OM1IYVWJw/X6Q175hChK9ylHsuLORipZd8uNjBNtr eaVROD4jNp8eoaCjKX+v+KrnDAnnFGh5THZoTf2Mn+XZC2bmooiX+YX8adCs3X6tI1Ni jf4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=j/5Jn7Rovm3p9y4iOyPLRnFhbvc8dPPQk+nWn+j3koc=; b=Q7wXMmMQ8F7GhVOCIPhNWlTAeQdng/cG/0xhnDyFw8l8wjMeWpbd/zW9jcknDl0Y5H V/yVuUXa1eHFWljo0fKDmvHM+LgCL8b3rrVhVuleieRAcS/zKuqHD7xEjVf62mtc5TWR OdQay7s4Zb62d9WyQpOfXzk39fyTNVNYvSc5DjX/ib1w0d4zR2zndcGPUR2eFSiPbSKL yPyCVpmmFsxV/djzXRihGwT7b5jD0wwzImXH7vSG5ENKjW26hI+qdUzwmQ7WPaA9mZ6P W4d5y1DJ6+E3TfAtAWL+hbYGbJ9eoHSkSITF3Rr7T+344j+GAn1JbuWNE8MS3JfUM2EG DDyA== X-Gm-Message-State: APjAAAUgJy38h/dUuKIYgqfdV5VLZTJwnBwwMDJV8Vwt8xfMy9GR47/6 06V4gF9d53TDnzRnrPmphlriZ5+ZhdI= X-Google-Smtp-Source: APXvYqyvt2qnYc6S6wJjpEn2jCSOVhjHm3zb6Q0i2fxF2KCkbFxKh5YP8fA4DqulTIVLHp5gF/isMQ== X-Received: by 2002:adf:bc84:: with SMTP id g4mr21719808wrh.135.1568069418088; Mon, 09 Sep 2019 15:50:18 -0700 (PDT) Received: from localhost.localdomain (ipbcc0f857.dynamic.kabel-deutschland.de. [188.192.248.87]) by smtp.gmail.com with ESMTPSA id b144sm844103wmb.3.2019.09.09.15.50.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Sep 2019 15:50:17 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Tue, 10 Sep 2019 00:50:02 +0200 Message-Id: <20190909225002.521-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190819215624.49795-7-andreas.rheinhardt@gmail.com> References: <20190819215624.49795-7-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v3 7/8] avformat/utils: Remove unnecessary packet copies X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 --- 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; } }