From patchwork Tue Aug 13 02:47:24 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 14463 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 738DC447E79 for ; Tue, 13 Aug 2019 05:48:56 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 617B268AC00; Tue, 13 Aug 2019 05:48:56 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 97F8568ABA8 for ; Tue, 13 Aug 2019 05:48:52 +0300 (EEST) Received: by mail-wr1-f67.google.com with SMTP id p17so106313257wrf.11 for ; Mon, 12 Aug 2019 19:48:52 -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=SWi5fh78ngA+k/rnSu9gbYO5GTV6MokMPcOXObW4Bnk=; b=D1p29oXIttgLRx+Nxr9W05Tjb9lTKzzVbqBsVAXMh2xqzLp/5xPIvF296WwpN4w/4F Lk2JLvPGcoNcXV8xp+AozTSxNWjNAcnYwRoHfl6b2NpGZN8jyVSp7Cm/DThp85xa3vI0 nkOcFi3vsyqPt1mNS4c7AwsdY34H4UdYy4KWvjWbEKDOwLVBouRNYquO+dKRuoDaTAYr //VGHt1uiX91dWDSYgfwjlnnFChzFFeverWExup6HYtJuYr1MRNhypflUYirph+qQb8D uUAipLYQtFOuECeoRqYPKhu6cQ0AzMbb75nJkshxo7TPV6RTz42lAFwGZ74B51dhaYyw DOcA== 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=SWi5fh78ngA+k/rnSu9gbYO5GTV6MokMPcOXObW4Bnk=; b=lHHXfsxGvXJ0TrqvU49i/chv5TH8N9+AYvZUZiIy2ni+HcD+KtRXgTIfGvxkppImXE O3Ip16Fk4nwn7TOslvhClBwbHqJPHDgcZUpJIZPSA/aBv22G+w8KOsAigQaTNxV7KBqp EOkqdtb+Ngjsp3ZtJbRs4R5MoTzzM4elxCuWeQeVB85rhij9af4d6MfhtANPSqnzetrI YaVgPfe52Su8IIcIes24wyin2GPkgd+qg9+maqXmVqofUkwWB7P4SL0hn+PC9eBgAwCA GtsvN9B1JVEVQv1ML60H6wh+e9MrSux9KxtnfmVICnw3GXOC8QJEgmaPAyRgZuHXZArk kcWg== X-Gm-Message-State: APjAAAU7Bw4oDjXS7f7XtGdVrvvJ2AD+DlQr8oXTez5aJ11m33l1bFGR GrVbSEyXmJgNG3diphYP3VWdRlEh X-Google-Smtp-Source: APXvYqxHQv0Ojj4WatdRTCEJ1WV2LTnByNpKlujWoJNqfIItYTPEEfLYVW1clgjJWmAigvJZjqEjqg== X-Received: by 2002:adf:d182:: with SMTP id v2mr15204463wrc.49.1565664531821; Mon, 12 Aug 2019 19:48:51 -0700 (PDT) Received: from localhost.localdomain (ipbcc06ceb.dynamic.kabel-deutschland.de. [188.192.108.235]) by smtp.gmail.com with ESMTPSA id k13sm25020613wro.97.2019.08.12.19.48.50 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 12 Aug 2019 19:48:51 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Tue, 13 Aug 2019 04:47:24 +0200 Message-Id: <20190813024726.6596-11-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190813024726.6596-1-andreas.rheinhardt@gmail.com> References: <20190813024726.6596-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 11/13] avformat/mux: Fix memleaks on errors II, improve documentation 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" 1. When an error happened in ff_interleave_add_packet during interleaving a packet for output, 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. 2. When no error happened in ff_interleave_add_packet during interleaving a packet for output, the input packet will already be blank (as if av_packet_unref had been applied to it), but it will nevertheless be zeroed and initialized again. This is unnecessary. Given the possibility of using other functions for interleavement than ff_interleave_packet_per_dts, this commit sets and documents what interleavement functions need to do: On success, the input packet (if given) should be a blank packet on return. On failure, cleaning up will be done by av_interleave_write_frame (where the already existing code for error handling can be reused). ff_audio_rechunk_interleave has been changed to abide by this. In order to keep these requirements consistent, they are kept in one place that is referenced by the other documentations. Updating the documentation incidentally also removed another reference to AVPacket's destructor. Signed-off-by: Andreas Rheinhardt --- libavformat/audiointerleave.c | 1 + libavformat/audiointerleave.h | 4 ++++ libavformat/avformat.h | 1 + libavformat/internal.h | 16 +++++++--------- libavformat/mux.c | 30 +++++++++++------------------- 5 files changed, 24 insertions(+), 28 deletions(-) diff --git a/libavformat/audiointerleave.c b/libavformat/audiointerleave.c index b7f4113b68..0c0dadf2f9 100644 --- a/libavformat/audiointerleave.c +++ b/libavformat/audiointerleave.c @@ -123,6 +123,7 @@ int ff_audio_rechunk_interleave(AVFormatContext *s, AVPacket *out, AVPacket *pkt aic->fifo_size = new_size; } av_fifo_generic_write(aic->fifo, pkt->data, pkt->size, NULL); + av_packet_unref(pkt); } else { // rewrite pts and dts to be decoded time line position pkt->pts = pkt->dts = aic->dts; diff --git a/libavformat/audiointerleave.h b/libavformat/audiointerleave.h index f28d5fefcc..99f427c05f 100644 --- a/libavformat/audiointerleave.h +++ b/libavformat/audiointerleave.h @@ -47,6 +47,10 @@ void ff_audio_interleave_close(AVFormatContext *s); * * @param get_packet function will output a packet when streams are correctly interleaved. * @param compare_ts function will compare AVPackets and decide interleaving order. + * + * Apart from these two functions, this function behaves like ff_interleave_packet_per_dts. + * + * @note This function must not be used with uncoded audio frames. */ int ff_audio_rechunk_interleave(AVFormatContext *s, AVPacket *out, AVPacket *pkt, int flush, int (*get_packet)(AVFormatContext *, AVPacket *, AVPacket *, int), diff --git a/libavformat/avformat.h b/libavformat/avformat.h index 92c3a89f4b..12aadc0ce7 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -558,6 +558,7 @@ typedef struct AVOutputFormat { /** * A format-specific function for interleavement. * If unset, packets will be interleaved by dts. + * See the description of ff_interleave_packet_per_dts for details. */ int (*interleave_packet)(struct AVFormatContext *, AVPacket *out, AVPacket *in, int flush); diff --git a/libavformat/internal.h b/libavformat/internal.h index d6a039c497..ea6f65fecf 100644 --- a/libavformat/internal.h +++ b/libavformat/internal.h @@ -232,9 +232,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, or < 0 on error. On success, pkt will be blank (as if unreferenced). */ int ff_interleave_add_packet(AVFormatContext *s, AVPacket *pkt, int (*compare)(AVFormatContext *, const AVPacket *, const AVPacket *)); @@ -497,15 +497,13 @@ int ff_framehash_write_header(AVFormatContext *s); int ff_read_packet(AVFormatContext *s, AVPacket *pkt); /** - * Interleave a packet per dts in an output media file. - * - * Packets with pkt->destruct == av_destruct_packet will be freed inside this - * function, so they cannot be used after it. Note that calling av_packet_unref() - * on them is still safe. + * Interleave an AVPacket per dts so it can be muxed. * - * @param s media file handle + * @param s an AVFormatContext for output. in resp. out will be added to + * resp. taken from its packet buffer. * @param out the interleaved packet will be output here - * @param pkt the input packet + * @param in the input packet. If not NULL will be a blank packet + * (as if unreferenced) on success. * @param flush 1 if no further packets are available as input and all * remaining packets should be output * @return 1 if a packet was output, 0 if no packet could be output, diff --git a/libavformat/mux.c b/libavformat/mux.c index c524c0886c..55fa0a184c 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -1164,20 +1164,13 @@ int ff_interleaved_peek(AVFormatContext *s, int stream, /** * Interleave an AVPacket correctly so it can be muxed. - * @param out the interleaved packet will be output here - * @param in the input packet - * @param flush 1 if no further packets are available as input and all - * remaining packets should be output - * @return 1 if a packet was output, 0 if no packet could be output, - * < 0 if an error occurred + * + * See the description of ff_interleave_packet_per_dts for details. */ static int interleave_packet(AVFormatContext *s, AVPacket *out, AVPacket *in, int flush) { if (s->oformat->interleave_packet) { - int ret = s->oformat->interleave_packet(s, out, in, flush); - if (in) - av_packet_unref(in); - return ret; + return s->oformat->interleave_packet(s, out, in, flush); } else return ff_interleave_packet_per_dts(s, out, in, flush); } @@ -1220,14 +1213,13 @@ int av_interleaved_write_frame(AVFormatContext *s, AVPacket *pkt) for (;; ) { AVPacket opkt; - int ret = interleave_packet(s, &opkt, pkt, flush); - if (pkt) { - memset(pkt, 0, sizeof(*pkt)); - av_init_packet(pkt); - pkt = NULL; - } - if (ret <= 0) //FIXME cleanup needed for ret<0 ? - return ret; + ret = interleave_packet(s, &opkt, pkt, flush); + if (ret < 0) + goto fail; + if (!ret) + return 0; + + pkt = NULL; ret = write_packet(s, &opkt); if (ret >= 0) @@ -1244,7 +1236,7 @@ fail: // This is a deviation from the usual behaviour // of av_interleaved_write_frame: We leave cleaning // up uncoded frames to write_uncoded_frame_internal. - if (!(pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) + if (pkt && !(pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) av_packet_unref(pkt); return ret; }