From patchwork Sat Apr 11 23:56:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 18882 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 AD41944AFC0 for ; Sun, 12 Apr 2020 02:56:57 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8BD6568B7E0; Sun, 12 Apr 2020 02:56:57 +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 8973D68B0FB for ; Sun, 12 Apr 2020 02:56:50 +0300 (EEST) Received: by mail-wr1-f65.google.com with SMTP id s8so6391739wrt.7 for ; Sat, 11 Apr 2020 16:56:50 -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:mime-version :content-transfer-encoding; bh=vUOmzUkhhv7DngjTFzAQmtY683mRoATYODKhM0ydkLw=; b=Oinc54HG1bqufrSaJOs75HMsYjt6v6AnYx0LPkmffoXVwvfKU5Nhl1AcjGlu+Py4ez NwfsUXGqQK4nh9U0mnvCX6+0fDdQK3Z6mQl3yGKS9FkkGOAdqxvMIXlw/urjr8oVgISq 1HMEjIaRD+0U9qLwkjyq3fwdTgpUsHPlU2XW9pUcTVOBfPxMxJ0fcCwYzTdXQTXh2Uqf AgjdtUcNzQe0Cc2h4mhbljsoGaH2zWtrrsJ4pVbxSv3ZJsAn4BSljIUfKn9K9/Wlx6SH 863wJHHNpXYEAFwAkSBQfvc+aC7v3f+8zBlMAMr9ge2jLwGcYCEm+kJrifHaeC18V9oH 1DTg== 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:mime-version :content-transfer-encoding; bh=vUOmzUkhhv7DngjTFzAQmtY683mRoATYODKhM0ydkLw=; b=k67ct1NXYMlOar9UERqGihrBcvMVSna5xEkQ3iGlRfK4sGOvhXLuJ1F7fSwSTBJuw2 rK66346wrbdkSWFHcDQDN2db+OSSh5aOWP/+ca4veuz/fH2Vn1BifSnfdOuLeqy8gkGF eGHWOOc/jU4ohdlfBawIY8cIgWPggxafz5c53Dm8WoAt4RmZ/1Xu4TfIGrtne7PstDu8 3kxOWjS52yCmoZ5e9W7Mo8au+D1/mlFSuUCvq9xVO5PqgCBzoW89ii/IET54E17i/isu BR1ZQI1mHL4091lKp9sXtxIzHahYuxH4HagWyaW1BBz2OyD7KjdHHMJegDmOamPvpiDj ahdA== X-Gm-Message-State: AGi0Pub+L1ia0qFb523hnAu6pFGtEroDME93yVEXb7PDAhkI+L53lXwz XU3YUaIekEcbTQEzvLD+UFQXraJb X-Google-Smtp-Source: APiQypK07SW/fpN8YxcuhOtVyRjwHxq7Qcu/MFvn9Qx6YlY1cOFq4nWA5llWmegPPxhLV4NT2flgtw== X-Received: by 2002:a5d:544f:: with SMTP id w15mr12454400wrv.77.1586649409556; Sat, 11 Apr 2020 16:56:49 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id s7sm8817442wrt.2.2020.04.11.16.56.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 11 Apr 2020 16:56:48 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sun, 12 Apr 2020 01:56:41 +0200 Message-Id: <20200411235642.18193-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 1/6] avformat/mux: Make uncoded frames av_packet_unref() compatible 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 , Marton Balint Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Currently uncoded frames (i.e. packets whose data actually points to an AVFrame) are not refcounted. As a consequence, calling av_packet_unref() on them will not free them, but may simply make sure that they leak by losing the pointer to the frame. This commit changes this by actually making uncoded frames refcounted. In order not to rely on sizeof(AVFrame) (which is not part of the public API and so must not be used here in libavformat) the packet's data is changed to a (padded) buffer containing just a pointer to an AVFrame. Said buffer is owned by an AVBuffer with a custom free function that frees the frame as well as the buffer. Thereby the pointer/the AVBuffer owns the AVFrame. Said ownership can actually be transferred by copying and resetting the pointer, as might happen when actually writing the uncoded frames in AVOutputFormat.write_uncoded_frame() (although currently no muxer makes use of this possibility). This makes packets containing uncoded frames compatible with av_packet_unref(). This already has three advantages in interleaved mode: 1. If an error happens at the preparatory steps (before the packet is put into the interleavement queue), the frame is properly freed. 2. If the trailer is never written, the frames still in the interleavement queue will now be properly freed by ff_packet_list_free(). 3. The custom code for moving the packet to the packet list in ff_interleave_add_packet() can be removed. It will also simplify fixing further memleaks in future commits. Suggested-by: Marton Balint Signed-off-by: Andreas Rheinhardt --- This added level of indirection even allowed to keep the possibility of complete ownership transfer to the muxer when writing uncoded frames. libavformat/mux.c | 52 +++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/libavformat/mux.c b/libavformat/mux.c index cc2d1e275a..78fe2336cb 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -550,12 +550,6 @@ fail: #define AV_PKT_FLAG_UNCODED_FRAME 0x2000 -/* Note: using sizeof(AVFrame) from outside lavu is unsafe in general, but - it is only being used internally to this file as a consistency check. - The value is chosen to be very unlikely to appear on its own and to cause - immediate failure if used anywhere as a real size. */ -#define UNCODED_FRAME_PACKET_SIZE (INT_MIN / 3 * 2 + (int)sizeof(AVFrame)) - #if FF_API_COMPUTE_PKT_FIELDS2 && FF_API_LAVF_AVCTX FF_DISABLE_DEPRECATION_WARNINGS @@ -650,7 +644,7 @@ static int compute_muxer_pkt_fields(AVFormatContext *s, AVStream *st, AVPacket * switch (st->codecpar->codec_type) { case AVMEDIA_TYPE_AUDIO: frame_size = (pkt->flags & AV_PKT_FLAG_UNCODED_FRAME) ? - ((AVFrame *)pkt->data)->nb_samples : + (*(AVFrame **)pkt->data)->nb_samples : av_get_audio_frame_duration(st->codec, pkt->size); /* HACK/FIXME, we skip the initial 0 size packets as they are most @@ -746,10 +740,10 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt) } if ((pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) { - AVFrame *frame = (AVFrame *)pkt->data; - av_assert0(pkt->size == UNCODED_FRAME_PACKET_SIZE); - ret = s->oformat->write_uncoded_frame(s, pkt->stream_index, &frame, 0); - av_frame_free(&frame); + AVFrame **frame = (AVFrame **)pkt->data; + av_assert0(pkt->size == sizeof(*frame)); + ret = s->oformat->write_uncoded_frame(s, pkt->stream_index, frame, 0); + av_packet_unref(pkt); } else { ret = s->oformat->write_packet(s, pkt); } @@ -926,14 +920,9 @@ int ff_interleave_add_packet(AVFormatContext *s, AVPacket *pkt, this_pktl = av_malloc(sizeof(AVPacketList)); if (!this_pktl) return AVERROR(ENOMEM); - if (pkt->flags & AV_PKT_FLAG_UNCODED_FRAME) { - av_assert0(pkt->size == UNCODED_FRAME_PACKET_SIZE); - av_assert0(((AVFrame *)pkt->data)->buf); - } else { - if ((ret = av_packet_make_refcounted(pkt)) < 0) { - av_free(this_pktl); - return ret; - } + if ((ret = av_packet_make_refcounted(pkt)) < 0) { + av_free(this_pktl); + return ret; } av_packet_move_ref(&this_pktl->pkt, pkt); @@ -1319,22 +1308,41 @@ int ff_write_chained(AVFormatContext *dst, int dst_stream, AVPacket *pkt, return ret; } +static void uncoded_frame_free(void *unused, uint8_t *data) +{ + av_frame_free((AVFrame **)data); + av_free(data); +} + static int write_uncoded_frame_internal(AVFormatContext *s, int stream_index, AVFrame *frame, int interleaved) { AVPacket pkt, *pktp; av_assert0(s->oformat); - if (!s->oformat->write_uncoded_frame) + if (!s->oformat->write_uncoded_frame) { + av_frame_free(&frame); return AVERROR(ENOSYS); + } if (!frame) { pktp = NULL; } else { + size_t bufsize = sizeof(frame) + AV_INPUT_BUFFER_PADDING_SIZE; + AVFrame **framep = av_mallocz(bufsize); + *framep = frame; pktp = &pkt; av_init_packet(&pkt); - pkt.data = (void *)frame; - pkt.size = UNCODED_FRAME_PACKET_SIZE; + pkt.buf = av_buffer_create((void *)framep, bufsize, + uncoded_frame_free, NULL, 0); + if (!pkt.buf) { + av_frame_free(framep); + av_free(framep); + return AVERROR(ENOMEM); + } + + pkt.data = (void *)framep; + pkt.size = sizeof(frame); pkt.pts = pkt.dts = frame->pts; pkt.duration = frame->pkt_duration; From patchwork Sat Apr 11 23:56:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 18883 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 9CF6344B711 for ; Sun, 12 Apr 2020 02:57:17 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8977E68B85D; Sun, 12 Apr 2020 02:57:17 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 0EE7F68B6F8 for ; Sun, 12 Apr 2020 02:57:11 +0300 (EEST) Received: by mail-wr1-f68.google.com with SMTP id s8so6392513wrt.7 for ; Sat, 11 Apr 2020 16:57:11 -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=tTLwnM4GvYx847sUiJtGfQQPUBgHZJilWF5UYZVEIXI=; b=Y5ZghDNmo6m8HJ+wG+Nq+mpxUg0OwJDg4lcQq2e5MKQlVtNOLoAqh1UPKr/cxiIh5i 2brV5U0Jzp3/2BCLcEFNRVKtY1+7yoPObRkDBJehmDKKGiXz+HKqrSvz2x40nx+bgRRH xuojPFzuP1dUJaKkKFd4DaZIesvhZUalfPIQ/YUR8xR0DUFMRslqj8Yn2YlEtne8669M JOI/Ti2uuYIWGkjvVPW7I23tyDH4oOCq6VJ9euXcQQntLqDVT5B3A4cWW5WACoTIQu2a YX/6cf9YVU1k+Xv3gi4lLkoLTTd8iVmn5S/VNvUhArbDf6wOaDsZ3Qyw6ObrurGHccWi KHyg== 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=tTLwnM4GvYx847sUiJtGfQQPUBgHZJilWF5UYZVEIXI=; b=qXu7d8+5iuHFPJ6PXDqZhxSfml0i3kb8W+yHN7CkEmDqsH8AOQ/fVZX66ScnYPbccf Ze5LVOGTgUqcPpz3/+ycNf8qBP0yWVSO3EZ/RYU3fM4XEGxE8J/RbN9OurMbUlJY4Z9Q lfoUHAmhvVKS6Oo7GvhL+wqmcflKFR4RGtcbAt4Swpi8XeCHuBVBQFR/ou4VK7oAe9BP disv4vjp5fz4LQiVGGSU1TZnQDZY4SHI9OpM8V0qO4fAjkPExtN03CU2NUvV69MNHZ0W Sk0rKo1fRWWshMp1USaZGmGLesXq7ippyPrSo4InaJ4mi1DK6l605jIm0juhXjA/H8rs FG3A== X-Gm-Message-State: AGi0PubmCavWqbanUebKJ8YXUZvrxbAclo4cYPHBLA5gPfxWze5LR6A7 D7jgptFAaqSWGTK/kDTyRZQR96Dl X-Google-Smtp-Source: APiQypK5oZjn6/4MeRBOshg1hJDQb1HJxi24bi6nMNGFOLObXn2elk9/kN0yFoKT+5FEh7VtgTygIA== X-Received: by 2002:a5d:6647:: with SMTP id f7mr9553370wrw.41.1586649430102; Sat, 11 Apr 2020 16:57:10 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id s7sm8817442wrt.2.2020.04.11.16.57.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 11 Apr 2020 16:57:09 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sun, 12 Apr 2020 01:56:42 +0200 Message-Id: <20200411235642.18193-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200411235642.18193-1-andreas.rheinhardt@gmail.com> References: <20200411235642.18193-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 2/6] avformat/mux: Fix leaks on error when writing noninterleaved uncoded frames 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" If writing uncoded frames in noninterleaved mode fails at the preparatory steps (i.e. before it reaches write_packet()), the packet would not be unreferenced and the frame would leak. This is fixed by unreferencing the packet in write_uncoded_frame_internal() instead. This also makes it possible to remove the unreferencing in write_packet() itself: In noninterleaved mode frames are now freed in write_uncoded_frame_internal(), while they are freed in interleaved mode when their containing packet gets unreferenced (like normal packets). Signed-off-by: Andreas Rheinhardt --- Just resending because of merge conflicts. libavformat/mux.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libavformat/mux.c b/libavformat/mux.c index 78fe2336cb..35b92edd27 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -743,7 +743,6 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt) AVFrame **frame = (AVFrame **)pkt->data; av_assert0(pkt->size == sizeof(*frame)); ret = s->oformat->write_uncoded_frame(s, pkt->stream_index, frame, 0); - av_packet_unref(pkt); } else { ret = s->oformat->write_packet(s, pkt); } @@ -1318,6 +1317,7 @@ 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) { @@ -1350,8 +1350,11 @@ static int write_uncoded_frame_internal(AVFormatContext *s, int stream_index, pkt.flags |= AV_PKT_FLAG_UNCODED_FRAME; } - return interleaved ? av_interleaved_write_frame(s, pktp) : - av_write_frame(s, pktp); + ret = interleaved ? av_interleaved_write_frame(s, pktp) : + av_write_frame(s, pktp); + if (pktp) + av_packet_unref(pktp); + return ret; } int av_write_uncoded_frame(AVFormatContext *s, int stream_index,