From patchwork Wed Apr 8 13:35:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 18780 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 92B1E449D17 for ; Wed, 8 Apr 2020 16:36:19 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7E29168B7D9; Wed, 8 Apr 2020 16:36:19 +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 04AAB68B7C8 for ; Wed, 8 Apr 2020 16:36:13 +0300 (EEST) Received: by mail-wr1-f68.google.com with SMTP id v5so7829587wrp.12 for ; Wed, 08 Apr 2020 06:36:12 -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=rfP84/y4cJ7dSMbFHmqXHrX/g07Z0a0kdOomQ2oaasg=; b=czdTkHHfpGZJap3kMq4yNldxv99pwK5TQDSutHdk93DQT/gQBd/abvgVrN/MD/BxX/ rkzhmke3+X1Kk5YEFWtpBtaykpY8HMDda9EF4khUwvNcXQZ6DA1PnT9+bGJRP1yJli5e bQbVkcBjO75rd1vyaPd16+iz22EKhw9dRLCCBEAikVFPBmXCdIP6z3oDoGQuEUqz3gF1 wNRRndn/D5Eh0VDgC5OY+4ybd/EN0G3lquFhINspNQ3JOnN2O/Xw37xQkiBnK6qql5Gb FZVZZSGYl0c/c9X0pvBOQJrFxF8PUWyiIpT12MYXd71aL5QeSCMr7CxFU8qpfI/XBLw4 8mzg== 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=rfP84/y4cJ7dSMbFHmqXHrX/g07Z0a0kdOomQ2oaasg=; b=Wj3jd9V9qE4edp2w9mI9UwXKncRM4BUHusTQD1QDwcdbOu3qE/NW3cI6GVxviTGD/O 4paSwXTubWxR9vH4j4h712Y7FstN5K8NOShMg+V9/tQ34xYdGYpndOzjR62L35iTeHcH mjzr7myOn48ylMZ1ECAFCb1B+63drR5GrkV28bCj0DqXer9yeQMqEHrhLOtaow2Vh2Wa nnmz0e64asesqtF2ZTXQch1ZDybbyAAl3+7sED9+ekVmPb3qdSCv+yn/u2OHE5l2N/lG cJOZ5eimRPr0bFQlGuw3QfRvBNFw+0F4liE9v+ezy8rPA+qZ+HsOmuR9csc47NsvbPcb gSSA== X-Gm-Message-State: AGi0PuZlaqgQWCb6Wvvo/IJJxeoGvGG1YRl7oqpiEzn/Uad8Z/dz27q4 OPW+DLSSCQ4pipqROsoWRHrpvUEN X-Google-Smtp-Source: APiQypJqZpTD9zj8ZPc7RsfkZ3fYgPn4g+He6RrIJoVOx7QepzP/1JSuBylfMxUTnDIXPb7nzogXQw== X-Received: by 2002:adf:ce07:: with SMTP id p7mr8262499wrn.261.1586352972044; Wed, 08 Apr 2020 06:36:12 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id 106sm26290270wrc.46.2020.04.08.06.36.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Apr 2020 06:36:11 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 8 Apr 2020 15:35:43 +0200 Message-Id: <20200408133544.18933-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200408133544.18933-1-andreas.rheinhardt@gmail.com> References: <20200408133544.18933-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 1/3] avformat/hlsenc: Fix memleak when deleting old segments 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" From: Andreas Rheinhardt Date: Wed, 8 Apr 2020 13:14:17 +0200 Subject: [PATCH 2/3] avformat/hlsenc: Use AVBPrint to avoid allocations of strings when deleting old segments. Signed-off-by: Andreas Rheinhardt --- Further todos for this function: 1. Allocating vtt_dirname_r in order to get vtt_dirname is actually a waste. One would need a dirname to AVBPrint function to avoid it. 2. replace_*_data_in_filename already work with AVBPrints internally. Refactor them to allow to directly output to a caller-supplied AVBPrint and use it to avoid allocating dirname_repl. libavformat/hlsenc.c | 57 +++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index 0f1f558590..d7562ad20c 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -489,18 +489,20 @@ static int hls_delete_old_segments(AVFormatContext *s, HLSContext *hls, HLSSegment *segment, *previous_segment = NULL; float playlist_duration = 0.0f; - int ret = 0, path_size, sub_path_size; + int ret = 0; int segment_cnt = 0; - char *dirname = NULL, *sub_path; + AVBPrint path; + char *dirname = NULL; char *dirname_r = NULL; char *dirname_repl = NULL; - char *path = NULL; char *vtt_dirname = NULL; char *vtt_dirname_r = NULL; AVDictionary *options = NULL; AVIOContext *out = NULL; const char *proto = NULL; + av_bprint_init(&path, 0, AV_BPRINT_SIZE_UNLIMITED); + segment = vs->segments; while (segment) { playlist_duration += segment->duration; @@ -550,73 +552,68 @@ static int hls_delete_old_segments(AVFormatContext *s, HLSContext *hls, while (segment) { av_log(hls, AV_LOG_DEBUG, "deleting old segment %s\n", segment->filename); - path_size = (hls->use_localtime_mkdir ? 0 : strlen(dirname)+1) + strlen(segment->filename) + 1; - path = av_malloc(path_size); - if (!path) { + if (!hls->use_localtime_mkdir) // segment->filename contains basename only + av_bprintf(&path, "%s%c", dirname, SEPARATOR); + av_bprintf(&path, "%s", segment->filename); + + if (!av_bprint_is_complete(&path)) { ret = AVERROR(ENOMEM); goto fail; } - if (hls->use_localtime_mkdir) - av_strlcpy(path, segment->filename, path_size); - else { // segment->filename contains basename only - snprintf(path, path_size, "%s%c%s", dirname, SEPARATOR, segment->filename); - } - proto = avio_find_protocol_name(s->url); if (hls->method || (proto && !av_strcasecmp(proto, "http"))) { av_dict_set(&options, "method", "DELETE", 0); - if ((ret = vs->avf->io_open(vs->avf, &out, path, AVIO_FLAG_WRITE, &options)) < 0) { + if ((ret = vs->avf->io_open(vs->avf, &out, path.str, + AVIO_FLAG_WRITE, &options)) < 0) { if (hls->ignore_io_errors) ret = 0; goto fail; } ff_format_io_close(vs->avf, &out); - } else if (unlink(path) < 0) { + } else if (unlink(path.str) < 0) { av_log(hls, AV_LOG_ERROR, "failed to delete old segment %s: %s\n", - path, strerror(errno)); + path.str, strerror(errno)); } if ((segment->sub_filename[0] != '\0')) { vtt_dirname_r = av_strdup(vs->vtt_avf->url); vtt_dirname = (char*)av_dirname(vtt_dirname_r); - sub_path_size = strlen(segment->sub_filename) + 1 + strlen(vtt_dirname) + 1; - sub_path = av_malloc(sub_path_size); - if (!sub_path) { + + av_bprint_clear(&path); + av_bprintf(&path, "%s%c%s", vtt_dirname, SEPARATOR, + segment->sub_filename); + av_freep(&vtt_dirname_r); + + if (!av_bprint_is_complete(&path)) { ret = AVERROR(ENOMEM); goto fail; } - snprintf(sub_path, sub_path_size, "%s%c%s", vtt_dirname, SEPARATOR, segment->sub_filename); - - av_freep(&vtt_dirname_r); - if (hls->method || (proto && !av_strcasecmp(proto, "http"))) { av_dict_set(&options, "method", "DELETE", 0); - if ((ret = vs->vtt_avf->io_open(vs->vtt_avf, &out, sub_path, AVIO_FLAG_WRITE, &options)) < 0) { + if ((ret = vs->vtt_avf->io_open(vs->vtt_avf, &out, path.str, + AVIO_FLAG_WRITE, &options)) < 0) { if (hls->ignore_io_errors) ret = 0; - av_freep(&sub_path); goto fail; } ff_format_io_close(vs->vtt_avf, &out); - } else if (unlink(sub_path) < 0) { + } else if (unlink(path.str) < 0) { av_log(hls, AV_LOG_ERROR, "failed to delete old segment %s: %s\n", - sub_path, strerror(errno)); + path.str, strerror(errno)); } - av_freep(&sub_path); } - av_freep(&path); + av_bprint_clear(&path); previous_segment = segment; segment = previous_segment->next; av_freep(&previous_segment); } fail: - av_freep(&path); + av_bprint_finalize(&path, NULL); av_freep(&dirname_r); av_freep(&dirname_repl); - av_freep(&vtt_dirname_r); return ret; }