From patchwork Sat Apr 11 21:19:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 18877 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 3EA8C44A8DB for ; Sun, 12 Apr 2020 00:20:40 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 24C7268B895; Sun, 12 Apr 2020 00:20:40 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 3228468B821 for ; Sun, 12 Apr 2020 00:20:32 +0300 (EEST) Received: by mail-wm1-f66.google.com with SMTP id h2so5811082wmb.4 for ; Sat, 11 Apr 2020 14:20:32 -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=doIcomMt6UqvFQY1t4MsbKpfpCkgbeZvdqOM+meyZBs=; b=Su+4h5s0NAwRJ8J9eYhZYR2sfPuaN27yVoK5j63E2q3WUcd55thUlPAz5dRWRz2cm7 uoZWnM2o0ds79GySGI3hVnjlXLKfGz4E4p7xPuR37yMHftyT3yEw5TU6/pzrerlXP4py V9DOD+3h+D+NW7Lk4by08XESX6BAKxwCbOjn+NP2uzleyZsAFPQQD9O08aCfsVYsVaFs vv5DmTM6of8//ky+H6k0TzFYnaZxOhX1XLnA2m35/vB4AQjxczljUqGZrtxQGE8zLyX2 +vBd3eeWXQKWzIa06GD9kY/fpeSZk0uuaBy99aWz9E20zQIO+yzmAxjFnNb/3lvYpxAX 0fzA== 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=doIcomMt6UqvFQY1t4MsbKpfpCkgbeZvdqOM+meyZBs=; b=GOjJ7IL79jKLjHJitin3gyz17rqFTAcBkNkqX0EYFkiSsIMiGsyMRPHi3dgBM0G5Wb LvhCENvKWWQ1S7TC5x8L6bqjYTllNIdJs0wbwPbsYyU7Fk8zT84BGvCtRYx9ftQI429b +QBRPlX73EIrf2yYCyIQs4AwWcaL5StKxEEwZB2EleZ9G9mwLuNafXn3P+hoLbEtVd92 wyETsQDPj1mHn51Af+5TcOHKx2vcqJv2lhuutU4oH9byiCZJWeecNScqkW+YEM89XnZD Cb1/EG+IaFGruw6iRGQpEMbxDq/kcw+S+FDi5gTLX4C8E2gYpMihZNw+yJ/MLR86h0rK WJbg== X-Gm-Message-State: AGi0Pub+0g40lmKkqmIAisi57mdsyOV7sInCKFP2WFOAhPr4ZB2SDC2K 5k5o9RIkCoRgnplCGUZftN3w0tSJ X-Google-Smtp-Source: APiQypLlXJ0PvCc6BnfxxXNcVevVgXgMVrw6BsWBRPYN1A5PISm2GvrmA6X7uXCpjwFEHOr1yYZtcA== X-Received: by 2002:a1c:cc16:: with SMTP id h22mr10859835wmb.47.1586640031272; Sat, 11 Apr 2020 14:20:31 -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.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 11 Apr 2020 14:20:30 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sat, 11 Apr 2020 23:19:52 +0200 Message-Id: <20200411211955.20843-3-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 3/6] avformat/mux: Fix leak when adding packet to interleavement queue fails 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" When an error happened in ff_interleave_add_packet() when adding a packet to the packet queue, said packet would not be unreferenced in ff_interleave_add_packet(), but would be zeroed in av_interleaved_write_frame(), which results in a memleak. This has been fixed: ff_interleave_add_packet() now always unreferences the input packet on error; as a result, it always returns blank packets which has been documented. Relying on this a call to av_packet_unref() in ff_audio_rechunk_interleave() can be removed. Signed-off-by: Andreas Rheinhardt --- This approach is way more robust than my old "let the caller clean up" approach. And given that uncoded frames are no longer as horrible as they are, one doesn't need to add further checks for whether one has an uncoded frame to free on error in ff_interleave_add_packet(). libavformat/audiointerleave.c | 4 +--- libavformat/internal.h | 4 ++-- libavformat/mux.c | 7 +++++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/libavformat/audiointerleave.c b/libavformat/audiointerleave.c index f10c83d7c9..36a3288242 100644 --- a/libavformat/audiointerleave.c +++ b/libavformat/audiointerleave.c @@ -136,10 +136,8 @@ int ff_audio_rechunk_interleave(AVFormatContext *s, AVPacket *out, AVPacket *pkt if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { AVPacket new_pkt; while ((ret = interleave_new_audio_packet(s, &new_pkt, i, flush)) > 0) { - if ((ret = ff_interleave_add_packet(s, &new_pkt, compare_ts)) < 0) { - av_packet_unref(&new_pkt); + if ((ret = ff_interleave_add_packet(s, &new_pkt, compare_ts)) < 0) return ret; - } } if (ret < 0) return ret; diff --git a/libavformat/internal.h b/libavformat/internal.h index 332477a532..e9d7d6754a 100644 --- a/libavformat/internal.h +++ b/libavformat/internal.h @@ -230,9 +230,9 @@ char *ff_data_to_hex(char *buf, const uint8_t *src, int size, int lowercase); int ff_hex_to_data(uint8_t *data, const char *p); /** - * Add packet to AVFormatContext->packet_buffer list, determining its + * Add packet to an AVFormatContext's packet_buffer list, determining its * interleaved position using compare() function argument. - * @return 0, or < 0 on error + * @return 0 on success, < 0 on error. pkt will always be blank on return. */ int ff_interleave_add_packet(AVFormatContext *s, AVPacket *pkt, int (*compare)(AVFormatContext *, const AVPacket *, const AVPacket *)); diff --git a/libavformat/mux.c b/libavformat/mux.c index e86214d585..f61dbd3c89 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -918,10 +918,13 @@ int ff_interleave_add_packet(AVFormatContext *s, AVPacket *pkt, int chunked = s->max_chunk_size || s->max_chunk_duration; this_pktl = av_malloc(sizeof(AVPacketList)); - if (!this_pktl) + if (!this_pktl) { + av_packet_unref(pkt); return AVERROR(ENOMEM); + } if ((ret = av_packet_make_refcounted(pkt)) < 0) { av_free(this_pktl); + av_packet_unref(pkt); return ret; } @@ -1216,7 +1219,7 @@ int av_interleaved_write_frame(AVFormatContext *s, AVPacket *pkt) av_init_packet(pkt); pkt = NULL; } - if (ret <= 0) //FIXME cleanup needed for ret<0 ? + if (ret <= 0) return ret; ret = write_packet(s, &opkt);