From patchwork Wed Sep 9 18:13:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 22240 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 AD47444B81C for ; Wed, 9 Sep 2020 21:13:24 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8472D68B954; Wed, 9 Sep 2020 21:13:24 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 0FA816804BA for ; Wed, 9 Sep 2020 21:13:18 +0300 (EEST) Received: by mail-wr1-f66.google.com with SMTP id s12so3945055wrw.11 for ; Wed, 09 Sep 2020 11:13:18 -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=zMpbqsnzZiHuSU1ZoGP47eDT8OnTuSUAkmkSR/SBugc=; b=JMvvcS4Sw57avZcrb/yBUBOPlwfA6avETsoSdNYKEqOiohkyJ6/OTuVKujceWCcAYK x2z4GcW6PtTpTk9XQbn3gQ9EMOHfpZMb4oHMrbiqvhTS8lMfAN+uxKj3RQOO+JbMl1BH 6Rkd22WiQ27QSfW+yK/rA1DMaCMuubVXcmfxgM6QiLcyTQ8xS8m43LFisF/EGDZQ15/z Xyer3YX/iuNi80Bc0Vfkdq0vvArYi2z/13x0SGl9QYjbnfca1B3zjh3O7gAGp/axMHVC BnXY2CVQH/a2OfgpFNVMSQDhpmd+Yde8K/zfrR0EEODA2wjjjGddwScel4HJih7hoUub eHng== 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=zMpbqsnzZiHuSU1ZoGP47eDT8OnTuSUAkmkSR/SBugc=; b=O311AGzM3q0zj/TxZ7kqdYiTj8KpR+7xNVqQWiv5OiTeEz65ROhcpEKNSjrqK8If9Z jRg8gX5Z43Jj4X8eosgCTeBhIvq6KEML/s3k4wTs3TUqKofK4Mml9xB61xlOP1t9Q8bU mApfPoBo6KUIZWs7Fal5ewPHSZ6Pl7QWRzutXSMciJ5SCZE5ZAj5Eteq1IjI5p9NRYIJ 23h/0eVye2As0As2wm/WOVy80vGpLlnzHPTdq6exSn4qCOgSB88lyqiIa/5l0xexg7r+ ZRiG7iVuUvkL8qXHKotfG8sQ9Uc52yoCIWqc05EQiX9WvQdjmy0C2fqHeNW5ZoGWXBse QGBg== X-Gm-Message-State: AOAM531qvJg3XNxhFMzMRPunyZlnrnwgfRPbhFJmCbe5e4Pvmj/JFv4/ qlWO7O8L2+KeIoFEsR5SimyojStx/5o= X-Google-Smtp-Source: ABdhPJyxkZOJuwCvQPjCNh9rGdlanVCpWt5CvK6jYXLp9+qwyptGxsHjfZHtV4QA22517SCDvM0BAA== X-Received: by 2002:a5d:4151:: with SMTP id c17mr5496851wrq.302.1599675197222; Wed, 09 Sep 2020 11:13:17 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1fb0f.dynamic.kabel-deutschland.de. [188.193.251.15]) by smtp.gmail.com with ESMTPSA id a83sm4989911wmh.48.2020.09.09.11.13.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Sep 2020 11:13:16 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 9 Sep 2020 20:13:07 +0200 Message-Id: <20200909181307.8006-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200907024952.11697-3-andreas.rheinhardt@gmail.com> References: <20200907024952.11697-3-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 3/10] avformat/segment: Fix segfault on allocation error, avoid allocation 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 the user has set none of the options specifying the segments' durations, a default value of 2s is used by duplicating a "2" string and using av_parse_time() on it. Yet duplicating the string was unchecked and if the allocation failed, one would get a segfault in av_parse_time(). This commit solves this by instead turning said option to use AV_OPT_TYPE_DURATION (which also uses av_parse_time() internally), avoiding duplicating the string altogether. Signed-off-by: Andreas Rheinhardt --- Updated to use the AV_OPT_TYPE_DURATION as has been discussed with Ridley Combs on IRC; the whole patchset (with the exception of a nit in the last patch) has been lgtm'ed and so I will push it in a few hours unless there are further objections. libavformat/segment.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/libavformat/segment.c b/libavformat/segment.c index 0c96c8c50c..e84dc7a426 100644 --- a/libavformat/segment.c +++ b/libavformat/segment.c @@ -90,7 +90,6 @@ typedef struct SegmentContext { char *entry_prefix; ///< prefix to add to list entry filenames int list_type; ///< set the list type AVIOContext *list_pb; ///< list file put-byte context - char *time_str; ///< segment duration specification string int64_t time; ///< segment duration int use_strftime; ///< flag to expand filename with strftime int increment_tc; ///< flag to increment timecode if found @@ -689,7 +688,7 @@ static int seg_init(AVFormatContext *s) "you can use output_ts_offset instead of it\n"); } - if (!!seg->time_str + !!seg->times_str + !!seg->frames_str > 1) { + if ((seg->time != 2000000) + !!seg->times_str + !!seg->frames_str > 1) { av_log(s, AV_LOG_ERROR, "segment_time, segment_times, and segment_frames options " "are mutually exclusive, select just one of them\n"); @@ -703,15 +702,6 @@ static int seg_init(AVFormatContext *s) if ((ret = parse_frames(s, &seg->frames, &seg->nb_frames, seg->frames_str)) < 0) return ret; } else { - /* set default value if not specified */ - if (!seg->time_str) - seg->time_str = av_strdup("2"); - if ((ret = av_parse_time(&seg->time, seg->time_str, 1)) < 0) { - av_log(s, AV_LOG_ERROR, - "Invalid time duration specification '%s' for segment_time option\n", - seg->time_str); - return ret; - } if (seg->use_clocktime) { if (seg->time <= 0) { av_log(s, AV_LOG_ERROR, "Invalid negative segment_time with segment_atclocktime option set\n"); @@ -1051,7 +1041,7 @@ static const AVOption options[] = { { "segment_atclocktime", "set segment to be cut at clocktime", OFFSET(use_clocktime), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, E}, { "segment_clocktime_offset", "set segment clocktime offset", OFFSET(clocktime_offset), AV_OPT_TYPE_DURATION, {.i64 = 0}, 0, 86400000000LL, E}, { "segment_clocktime_wrap_duration", "set segment clocktime wrapping duration", OFFSET(clocktime_wrap_duration), AV_OPT_TYPE_DURATION, {.i64 = INT64_MAX}, 0, INT64_MAX, E}, - { "segment_time", "set segment duration", OFFSET(time_str),AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, E }, + { "segment_time", "set segment duration", OFFSET(time),AV_OPT_TYPE_DURATION, {.i64 = 2000000}, INT64_MIN, INT64_MAX, E }, { "segment_time_delta","set approximation value used for the segment times", OFFSET(time_delta), AV_OPT_TYPE_DURATION, {.i64 = 0}, 0, INT64_MAX, E }, { "segment_times", "set segment split time points", OFFSET(times_str),AV_OPT_TYPE_STRING,{.str = NULL}, 0, 0, E }, { "segment_frames", "set segment split frame numbers", OFFSET(frames_str),AV_OPT_TYPE_STRING,{.str = NULL}, 0, 0, E }, From patchwork Wed Sep 9 18:23:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 22241 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 EAB3844B8AC for ; Wed, 9 Sep 2020 21:24:02 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id BE3F568B94A; Wed, 9 Sep 2020 21:24:02 +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 72E1E68B4A9 for ; Wed, 9 Sep 2020 21:23:56 +0300 (EEST) Received: by mail-wr1-f68.google.com with SMTP id c18so3981950wrm.9 for ; Wed, 09 Sep 2020 11:23:56 -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=6S9FGdTYWYThhZShvcKAOGLcLaLrnPzofCid2wBDbUQ=; b=A3hy2XACRU98GDn8uXNQCbOqDlSb7ubgocNr/LcejQvutqttAeTF/XmjtV47SLVJy8 iMgyeWV0Hy2ffrZmCCNfO+ei95O5qqR+lGpGU2uCrYXWOl4cDcJ8m719s37YjzBMR8gF lxBVMTmRXrTHISnu+/O/r3f3+i6sRoe4z3UlgNk0f3kdlCVrEcyn85FqqtJLFBGircnm o5lUr+ZU/UfkP42nWZIei2QQscg9iM75LEkOaCouDZRYQbcSIonystnlsYNeAxsUIH35 NZZJH20U6dkMIqfOMd77BZVsDKCJk5ZGxHqUvgT1QoeBNtgl9nIgTBLtZ1CiJNbu5JuQ TtGw== 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=6S9FGdTYWYThhZShvcKAOGLcLaLrnPzofCid2wBDbUQ=; b=Bfg0aEcngp8uvoM3rXQftTMMIC53PVRijYZWnupwmf+yk4rAJ0KMPNTE0IjWgDASAq jTZOnOnPgIhCm45CX4v1/wXufNItzC6QjZB+XvaFGi3Dt1wRGVGBSjdKrd8m38cuzLMo tB8KBYytZ+wUzHpLzU5KGUMTyLjCCVPCm2jEYfcBee0p8h05934cmzu8YJH52Qnd+cFO jJpRHDgjZhefuSqr7iAAnG6ZkFp5NwUTAVj/+ycrkIgiHNj7mM0oM/7gUk9spl40bxuE 9oBzA4eqBbhtJMxgHmzBcsNE3N9IEVO0t33inT8gUe1yHP5s+UjHyrVO97pPSbswuRvz vNLg== X-Gm-Message-State: AOAM533sxjQtKrxk2p1wVzYCwkvVdazu86cFX8pWefrCMsJZ/qk4334K EoC9gFyL0vsdFDPBvc3jgbxiv9hGGFs= X-Google-Smtp-Source: ABdhPJz1nHtsZl/OcLFudbRZKvOLc0LNx+Xcg6hifVzyL2ZmIx/C/X172bfDTCo+3xJSOO77sj9vlA== X-Received: by 2002:a5d:6caf:: with SMTP id a15mr5147826wra.344.1599675835594; Wed, 09 Sep 2020 11:23:55 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1fb0f.dynamic.kabel-deutschland.de. [188.193.251.15]) by smtp.gmail.com with ESMTPSA id c10sm4919013wmk.30.2020.09.09.11.23.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Sep 2020 11:23:55 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 9 Sep 2020 20:23:47 +0200 Message-Id: <20200909182347.8540-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200907024952.11697-10-andreas.rheinhardt@gmail.com> References: <20200907024952.11697-10-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 10/10] avformat/segment: Avoid duplicating string when parsing frames list 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" Signed-off-by: Andreas Rheinhardt --- Now no longer setting a pointer to one past the end of a string (although this is legal as long as one doesn't read from it). libavformat/segment.c | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/libavformat/segment.c b/libavformat/segment.c index 8e3f47d96a..dff3d0ed48 100644 --- a/libavformat/segment.c +++ b/libavformat/segment.c @@ -524,46 +524,40 @@ end: static int parse_frames(void *log_ctx, int **frames, int *nb_frames, const char *frames_str) { - char *p; - int i, ret = 0; - char *frames_str1 = av_strdup(frames_str); - char *saveptr = NULL; - - if (!frames_str1) - return AVERROR(ENOMEM); - -#define FAIL(err) ret = err; goto end + const char *p; + int i; *nb_frames = 1; - for (p = frames_str1; *p; p++) + for (p = frames_str; *p; p++) if (*p == ',') (*nb_frames)++; *frames = av_malloc_array(*nb_frames, sizeof(**frames)); if (!*frames) { av_log(log_ctx, AV_LOG_ERROR, "Could not allocate forced frames array\n"); - FAIL(AVERROR(ENOMEM)); + return AVERROR(ENOMEM); } - p = frames_str1; + p = frames_str; for (i = 0; i < *nb_frames; i++) { long int f; char *tailptr; - char *fstr = av_strtok(p, ",", &saveptr); - p = NULL; - if (!fstr) { + if (*p == '\0' || *p == ',') { av_log(log_ctx, AV_LOG_ERROR, "Empty frame specification in frame list %s\n", frames_str); - FAIL(AVERROR(EINVAL)); + return AVERROR(EINVAL); } - f = strtol(fstr, &tailptr, 10); - if (*tailptr || f <= 0 || f >= INT_MAX) { + f = strtol(p, &tailptr, 10); + if (*tailptr != '\0' && *tailptr != ',' || f <= 0 || f >= INT_MAX) { av_log(log_ctx, AV_LOG_ERROR, "Invalid argument '%s', must be a positive integer < INT_MAX\n", - fstr); - FAIL(AVERROR(EINVAL)); + p); + return AVERROR(EINVAL); } + if (*tailptr == ',') + tailptr++; + p = tailptr; (*frames)[i] = f; /* check on monotonicity */ @@ -571,13 +565,11 @@ static int parse_frames(void *log_ctx, int **frames, int *nb_frames, av_log(log_ctx, AV_LOG_ERROR, "Specified frame %d is smaller than the last frame %d\n", (*frames)[i], (*frames)[i-1]); - FAIL(AVERROR(EINVAL)); + return AVERROR(EINVAL); } } -end: - av_free(frames_str1); - return ret; + return 0; } static int open_null_ctx(AVIOContext **ctx)