From patchwork Mon Apr 5 01:44:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 26741 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 C04254481E1 for ; Mon, 5 Apr 2021 04:44:50 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 98D0E687FBB; Mon, 5 Apr 2021 04:44:50 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from EUR06-VI1-obe.outbound.protection.outlook.com (mail-vi1eur06olkn2025.outbound.protection.outlook.com [40.92.17.25]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 9D095687FBB for ; Mon, 5 Apr 2021 04:44:43 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Mdfy59D6gzx9Qlwb/yCDTlJhjAU+FPCrdiXZOvlSChJV9Wxu0zqQILOI9YqztxIo+iNzyaSDXFUTrMyk/Mbgs5cd3AkP/V2Re3UYtjjq8xSlR6svWFECSw61jSGzjLLasyt1PiOFlPQSNhL/a2y36Jox/RSeUb33aTtbnzA5S0TQMGalhdN98TD2wbMfpTYXd/cT7cd4ccNGZWotxP/NY//rv0H0rm2IdZiDcS0KjLdZmevyKxMPi8MIkbqWdQTy9xeDey//4OAzlxjrSYWouiByWDEY7dt9eVERbroMjN/xFTbfZwPmNd9Qj1IzvFSjXHzZNzdvCksVAo/6+xGdLA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=sTr4mY643T0L9DuHOzQGt3cx6gb/UkkigUMYGFGH2ms=; b=NQxQtElSjxT6TUr+VofSZKXH+C9V1oV3kRFtQXgsQXIhFf3lUm9WJSLT/LFj/in9rUkRKriGMRFpI8oUEHFgFYHuvPINyFaoEmhxz8v4bqPgABFPd96+nj10ergq6jnss6y/mffYP2Mooed3XolvQJdr2ANMIljmFlbNwaSQUlGsPOExchY4sq3Z3i5458htcMVEuDlgMFarGYFHwsOZ5XS8uZlRnwM3IVv+zsQqDMeABFd8QbPyyoPNeRh2dJxi5AAuyWqq9t0wkOADHGtrAoCuQ+VaXP0UZ37iN67PEnZfBPz2xbZyjY9er3P7YACx6j4N04gKNLIEIdkFaglj7g== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=sTr4mY643T0L9DuHOzQGt3cx6gb/UkkigUMYGFGH2ms=; b=nOIr0MzPajgndl6RLNe/N1ZGV59Cv9zWEsBZKd59FirmZIdiX5nBDKJHcSzQZinmeIyi0snTcg1M7eobHjf+9FJUH2o+9BpmIcP3cCROkIqM64fwn3mXC24sEZZS+egiNHRI6Ce/eJWI5D1+evJHXGGl5ibu38O9KYYJTdgCvBejiFXhXib9WVnDFxFeOuoo2u225MnsWJMif+teC/m5/hsUGo0YY81qeW32+F5eYSsjq6YuRYUnCzcXuNeWpsnZOZqqxML4O/VHpVnP7f6R9XPGaodN51Mr6S4WtdSjXlgy+9ypUDRAko9/FKmXzyC3bBht8YKFEJ0BQvqMH/7Qgw== Received: from AM7EUR06FT063.eop-eur06.prod.protection.outlook.com (2a01:111:e400:fc36::4b) by AM7EUR06HT025.eop-eur06.prod.protection.outlook.com (2a01:111:e400:fc36::210) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3999.28; Mon, 5 Apr 2021 01:44:42 +0000 Received: from HE1PR0301MB2154.eurprd03.prod.outlook.com (2a01:111:e400:fc36::52) by AM7EUR06FT063.mail.protection.outlook.com (2a01:111:e400:fc36::403) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3999.28 via Frontend Transport; Mon, 5 Apr 2021 01:44:42 +0000 X-IncomingTopHeaderMarker: OriginalChecksum:0F384152DE747CCEB13776E6931A867B176A3CDC8A5954025661B8F6E27C6611; UpperCasedChecksum:50EB8DCD41946C455705EEB41249D4DC3E744E201DBC3B1667ACF1D28544237E; SizeAsReceived:7591; Count:48 Received: from HE1PR0301MB2154.eurprd03.prod.outlook.com ([fe80::8128:5de5:4e94:9a21]) by HE1PR0301MB2154.eurprd03.prod.outlook.com ([fe80::8128:5de5:4e94:9a21%3]) with mapi id 15.20.3999.032; Mon, 5 Apr 2021 01:44:41 +0000 From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Mon, 5 Apr 2021 03:44:30 +0200 Message-ID: X-Mailer: git-send-email 2.27.0 In-Reply-To: References: X-TMN: [59JWNkDO2VkPSHJ47xSRiKYxGKPItWcZ] X-ClientProxiedBy: AM0PR02CA0189.eurprd02.prod.outlook.com (2603:10a6:20b:28e::26) To HE1PR0301MB2154.eurprd03.prod.outlook.com (2603:10a6:3:2a::22) X-Microsoft-Original-Message-ID: <20210405014434.3973535-1-andreas.rheinhardt@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from sblaptop.fritz.box (188.192.137.96) by AM0PR02CA0189.eurprd02.prod.outlook.com (2603:10a6:20b:28e::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3999.28 via Frontend Transport; Mon, 5 Apr 2021 01:44:41 +0000 X-MS-PublicTrafficType: Email X-IncomingHeaderCount: 48 X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-Correlation-Id: 8feb461c-a0b9-4426-d20e-08d8f7d46187 X-MS-TrafficTypeDiagnostic: AM7EUR06HT025: X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: LhKiZ1/OTKZOnqTWn7jhG1fFMwwLLvQlLq+x/yJbOrKKOn82aFz0Ef8GtnVu3CUguJS6nFFwh30fY5sM4HdpuXoHbTKAon/RtVg445k9eJQvgLAPE2vLnTx6K5nxOLIJVQlwrLH+ZyEKc7+k9SiFACfNzMZEgQfMpL1/LP1H9KlD3A96LFe9IrfS7GLPnk6kSUnDTR0nvS6g3mnM9EPauspJuWje8cgNK1DVO4Nl8tonL3lKeEy52+Yepz3xZ0/VCFJGEsUUoj3MyKpl12xgPAAH8u9Z1r9MPx8mutCLgfzzxPt+dHysjDLwlVjbaUIEVTGcGZyhdix+x6XksJh9j2EjNZ5QyMEcd3aGme+HPSMmtMV/ErBMMR84Y2RnE0jm X-MS-Exchange-AntiSpam-MessageData: JUkfkFd01W324/cJflFEdCduGFys8jApPWBGKhRLLyJypqsT5ugmM2Fy4oC8bocsqhmk4sy2mvp0Km2bcBv8sxnMxxyZSelW/m0UCqCt6p1lGGgWxTRLWJiyy97IgESN8wYjfip3+bB0sql0B+kPBw== X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8feb461c-a0b9-4426-d20e-08d8f7d46187 X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Apr 2021 01:44:41.6222 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-AuthSource: AM7EUR06FT063.eop-eur06.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: Internet X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM7EUR06HT025 Subject: [FFmpeg-devel] [PATCH 2/6] avcodec/mpegvideo: Fix memleak upon allocation error 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 When slice-threading is used, ff_mpv_common_init() duplicates the first MpegEncContext and allocates some buffers for each MpegEncContext (the first as well as the copies). But the count of allocated MpegEncContexts is not updated until after everything has been allocated and if an error happens after the first one has been allocated, only the first one is freed; the others leak. This commit fixes this: The count is now set before the copies are allocated. Furthermore, the copies are now created and initialized before the first MpegEncContext, so that the buffers exclusively owned by each MpegEncContext are still NULL in the src MpegEncContext so that no double-free happens upon allocation failure. Given that this effectively touches every line of the init code, it has also been factored out in a function of its own in order to remove code duplication with the same code in ff_mpv_common_frame_size_change() (which was never called when using more than one slice (and if it were, there would be potential double-frees)). Signed-off-by: Andreas Rheinhardt --- libavcodec/mpegvideo.c | 91 +++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 54 deletions(-) diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c index 2351a09874..7327204e99 100644 --- a/libavcodec/mpegvideo.c +++ b/libavcodec/mpegvideo.c @@ -366,13 +366,6 @@ static int init_duplicate_context(MpegEncContext *s) if (s->mb_height & 1) yc_size += 2*s->b8_stride + 2*s->mb_stride; - s->sc.edge_emu_buffer = - s->me.scratchpad = - s->me.temp = - s->sc.rd_scratchpad = - s->sc.b_scratchpad = - s->sc.obmc_scratchpad = NULL; - if (s->encoding) { if (!FF_ALLOCZ_TYPED_ARRAY(s->me.map, ME_MAP_SIZE) || !FF_ALLOCZ_TYPED_ARRAY(s->me.score_map, ME_MAP_SIZE)) @@ -413,6 +406,35 @@ static int init_duplicate_context(MpegEncContext *s) return 0; } +/** + * Initialize an MpegEncContext's thread contexts. Presumes that + * slice_context_count is already set and that all the fields + * that are freed/reset in free_duplicate_context() are NULL. + */ +static int init_duplicate_contexts(MpegEncContext *s) +{ + int nb_slices = s->slice_context_count, ret; + + /* We initialize the copies before the original so that + * fields allocated in init_duplicate_context are NULL after + * copying. This prevents double-frees upon allocation error. */ + for (int i = 1; i < nb_slices; i++) { + s->thread_context[i] = av_memdup(s, sizeof(MpegEncContext)); + if (!s->thread_context[i]) + return AVERROR(ENOMEM); + if ((ret = init_duplicate_context(s->thread_context[i])) < 0) + return ret; + s->thread_context[i]->start_mb_y = + (s->mb_height * (i ) + nb_slices / 2) / nb_slices; + s->thread_context[i]->end_mb_y = + (s->mb_height * (i + 1) + nb_slices / 2) / nb_slices; + } + s->start_mb_y = 0; + s->end_mb_y = nb_slices > 1 ? (s->mb_height + nb_slices / 2) / nb_slices + : s->mb_height; + return init_duplicate_context(s); +} + static void free_duplicate_context(MpegEncContext *s) { if (!s) @@ -949,29 +971,12 @@ av_cold int ff_mpv_common_init(MpegEncContext *s) s->context_initialized = 1; memset(s->thread_context, 0, sizeof(s->thread_context)); s->thread_context[0] = s; + s->slice_context_count = nb_slices; // if (s->width && s->height) { - if (nb_slices > 1) { - for (i = 0; i < nb_slices; i++) { - if (i) { - s->thread_context[i] = av_memdup(s, sizeof(MpegEncContext)); - if (!s->thread_context[i]) - goto fail_nomem; - } - if ((ret = init_duplicate_context(s->thread_context[i])) < 0) - goto fail; - s->thread_context[i]->start_mb_y = - (s->mb_height * (i) + nb_slices / 2) / nb_slices; - s->thread_context[i]->end_mb_y = - (s->mb_height * (i + 1) + nb_slices / 2) / nb_slices; - } - } else { - if ((ret = init_duplicate_context(s)) < 0) - goto fail; - s->start_mb_y = 0; - s->end_mb_y = s->mb_height; - } - s->slice_context_count = nb_slices; + ret = init_duplicate_contexts(s); + if (ret < 0) + goto fail; // } return 0; @@ -1079,7 +1084,7 @@ int ff_mpv_common_frame_size_change(MpegEncContext *s) &s->chroma_x_shift, &s->chroma_y_shift); if (err < 0) - return err; + goto fail; if ((err = init_context_frame(s))) goto fail; @@ -1088,31 +1093,9 @@ int ff_mpv_common_frame_size_change(MpegEncContext *s) s->thread_context[0] = s; if (s->width && s->height) { - int nb_slices = s->slice_context_count; - if (nb_slices > 1) { - for (i = 0; i < nb_slices; i++) { - if (i) { - s->thread_context[i] = av_memdup(s, sizeof(MpegEncContext)); - if (!s->thread_context[i]) { - err = AVERROR(ENOMEM); - goto fail; - } - } - if ((err = init_duplicate_context(s->thread_context[i])) < 0) - goto fail; - s->thread_context[i]->start_mb_y = - (s->mb_height * (i) + nb_slices / 2) / nb_slices; - s->thread_context[i]->end_mb_y = - (s->mb_height * (i + 1) + nb_slices / 2) / nb_slices; - } - } else { - err = init_duplicate_context(s); - if (err < 0) - goto fail; - s->start_mb_y = 0; - s->end_mb_y = s->mb_height; - } - s->slice_context_count = nb_slices; + err = init_duplicate_contexts(s); + if (err < 0) + goto fail; } return 0;