From patchwork Sat Apr 11 21:19:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 18879 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 1F6C044A8DB for ; Sun, 12 Apr 2020 00:20:43 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 08A2E68B89A; Sun, 12 Apr 2020 00:20:43 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 2F30568B4AC for ; Sun, 12 Apr 2020 00:20:34 +0300 (EEST) Received: by mail-wm1-f68.google.com with SMTP id f20so6148529wmh.3 for ; Sat, 11 Apr 2020 14:20:34 -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=OkE/Bf7XusloCvT8nBJTvGOYanhB5LOvauoXiUCnoUo=; b=SKDl2dZsHqsA2a2A6kVvg3QuG1srpDTQY8dVJI4gkdymiWsiJ8T8G6x0gDP6auW0IG slF089JlaNi4pAYWsABNtUcjp7P7s0GfPCumbOGs3kNwDNmGYrTxsLQIENicLlGgRxKi D4uIfb/oYjQI2kUw/9Oo/4mCcWJTW/S0810UYWnO07v73cKgIvzqAJ+R0Cazs+49IF8l 1NFaeVi1ZqQQpE/n4O/vjMZV5r6/GPm0kHwga1Xn95mvGOrLTCSoyuUE4YlKssPt4bI4 CKQTpsOEcHfedZrmlA2Ps5Mwa5lqC214nwm+bMchKPWAlKa2f5gmyRkhVtUsRUeqlfEk zTcg== 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=OkE/Bf7XusloCvT8nBJTvGOYanhB5LOvauoXiUCnoUo=; b=bgC/DAUQnvqftYblG3fTflikzOaxmMqmT64Ud+UdZP2xjEvKPgC5mNboQPh3D/+iu3 ev3g5/ivHFj37+fycv97ZxanRjV9195y9p5wWMGzfRppnGB7XwJF2IIsFRwxWo2yG/Do GX3ipDA3VxyTwSVANCDnR/ee52KOLoljvSS3RxlYJ2iTVmZyjY3AVtSO1ok427XtYp58 IizKXnccMwtZUA+ijPGk4GCeE9RPExgza138Fukitpu8ieO0DgXh4Hq2NdvZSdXqERoB qT2qQ3tw3KdefJOwyQu7RF8vwrDa6W89A+/sImYMwMR2T5M6xLQ94GXuE3BZIwtP0vF0 Fr6Q== X-Gm-Message-State: AGi0PuZL3pNMslEvg7Au5SL1vkbZdFgTKW1XaeiElGQ66wWbP85a55kL UC2cAo+cPyTiGILj/D455HI+PWun X-Google-Smtp-Source: APiQypKPpkkBMguIJ1SnpcMKSOYJJgwWVN/V42W4QtG+6G786FeoPA5D7Kifer5Ef6PE4Fa+WU3Lzw== X-Received: by 2002:a7b:cb03:: with SMTP id u3mr10692323wmj.12.1586640033268; Sat, 11 Apr 2020 14:20:33 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id h2sm7820650wmb.16.2020.04.11.14.20.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 11 Apr 2020 14:20:32 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sat, 11 Apr 2020 23:19:54 +0200 Message-Id: <20200411211955.20843-5-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200411211955.20843-1-andreas.rheinhardt@gmail.com> References: <20200411211955.20843-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 5/6] avformat/mux: Don't modify packets we don't own 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" 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 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, a blank 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. Notice that packets that contain uncoded frames are exempt from this because these packets are not owned by and returned to the user. This also moves unreferencing the packets containing uncoded frames to av_write_frame() in the noninterleaved codepath; in the interleaved codepath, these packets are already freed in av_interleaved_write_frame(), so that unreferencing the packets in write_uncoded_frame_internal() is no longer needed. It has been removed. Signed-off-by: Andreas Rheinhardt --- I was actually surprised when I realized how freeing uncoded frames in the noninterleaved codepath could be left to av_write_frame(). libavformat/mux.c | 48 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/libavformat/mux.c b/libavformat/mux.c index cae9f42d11..48c0d4cd5f 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -874,11 +874,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); @@ -889,22 +890,49 @@ int av_write_frame(AVFormatContext *s, AVPacket *pkt) return 1; } + if (in->flags & AV_PKT_FLAG_UNCODED_FRAME) { + pkt = in; + } else { + /* 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->buf = NULL; + pkt->data = in->data; + pkt->size = in->size; + ret = av_packet_copy_props(pkt, in); + if (ret < 0) + return ret; + if (in->buf) { + pkt->buf = av_buffer_ref(in->buf); + if (!pkt->buf) { + ret = AVERROR(ENOMEM); + 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: + // Uncoded frames using the noninterleaved codepath are freed here + av_packet_unref(pkt); + return ret; } #define CHUNK_START 0x1000 @@ -1319,7 +1347,6 @@ static int write_uncoded_frame_internal(AVFormatContext *s, int stream_index, AVFrame *frame, int interleaved) { AVPacket pkt, *pktp; - int ret; av_assert0(s->oformat); if (!s->oformat->write_uncoded_frame) { @@ -1349,11 +1376,8 @@ static int write_uncoded_frame_internal(AVFormatContext *s, int stream_index, pkt.flags |= AV_PKT_FLAG_UNCODED_FRAME; } - ret = interleaved ? av_interleaved_write_frame(s, pktp) : - av_write_frame(s, pktp); - if (pktp) - av_packet_unref(pktp); - return ret; + return interleaved ? av_interleaved_write_frame(s, pktp) : + av_write_frame(s, pktp); } int av_write_uncoded_frame(AVFormatContext *s, int stream_index,