From patchwork Fri Apr 17 10:41:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Josh Brewster X-Patchwork-Id: 19024 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 22A8444B06E for ; Fri, 17 Apr 2020 13:41:09 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id F191A68BBB3; Fri, 17 Apr 2020 13:41:08 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail4.protonmail.ch (mail4.protonmail.ch [185.70.40.27]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A480A68B8F5 for ; Fri, 17 Apr 2020 13:41:07 +0300 (EEST) Date: Fri, 17 Apr 2020 10:41:00 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=default; t=1587120066; bh=TVvqRHmtiayFJ5GmUKpbeAhzBFhBtASogiiZQtx4V78=; h=Date:To:From:Reply-To:Subject:From; b=p7JCcS3/dgQo6k2C1tAt2UKugvIHvB4iCcMDWd4y2TyYQIrsr3qOfsGlI3VjFJhEd FD44PxtEZt4dMOP5WL+vDzHPITaXQfXJ/FzRkjZOJlYMcDziUVtKaBoXNfjaTspPnQ 00kye6z9cMN8PuMDdXC6YkTEV0Fx9sBINX68Izzk= To: FFmpeg development discussions and patches From: Josh Brewster Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-1.2 required=7.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM shortcircuit=no autolearn=disabled version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on mail.protonmail.ch Subject: [FFmpeg-devel] [PATCH v2] libavcodec/libx264: fix reference frame computation based on level 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" > Hi, > > > From: ffmpeg-devel ffmpeg-devel-bounces@ffmpeg.org On Behalf Of > > josh.brewster@protonmail.com > > Sent: Friday, April 17, 2020 07:05 > > To: ffmpeg-devel@ffmpeg.org > > Subject: [FFmpeg-devel] [PATCH] libavcodec/libx264: fix reference frame > > computation based on level > > Hell, can someone please review this patch? It fixes a wrong reference frame > > computation problem when using parameters such as "-level 31" instead of > > "-level 3.1". > > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c > > index a08fe0ce76..1149f2d668 100644 > > --- a/libavcodec/libx264.c > > +++ b/libavcodec/libx264.c > > @@ -701,11 +701,14 @@ FF_ENABLE_DEPRECATION_WARNINGS > > > > if (!strcmp(x4->level, "1b")) { > > level_id = 9; > > > > > > - } else if (strlen(x4->level) <= 3){ > > > > > > > > - } else if (av_strtod(x4->level, NULL) < 7){ > > level_id = av_strtod(x4->level, &tail) * 10 + 0.5; > > if (*tail) > > level_id = -1; > > } > > > > > > - else { > > > > > > Wrong coding style for "else", should be "} else {" > > > - level_id = av_strtod(x4->level, NULL); > > > > > > - } > > if (level_id <= 0) > > av_log(avctx, AV_LOG_WARNING, "Failed to parse level\\n"); > > > > > > This part of code of parsing level_id seems redundant, since PARSE_X264_OPT("level", level) has > been called and did the same thing inside libx264, which converts x4->level to x4->params.i_level_idc > correctly (equals 31), regardless of "-level 3.1 or level 31". > > Hence it would be better to use x4->params.i_level_idc directly. > > - Linjie > > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". Hi Linjie, thanks for the feedback. I have changed the patch to use the right parameter. I only made sure that the level was positive because its initial value was -1. > else if (x4->params.i_level_idc >= 0) { Let me know if I need to reject 0 too. It seemed like premature optimization as the level simply wouldn't be present in x264_levels. From 98344809c22d574959eb4351a1cc03f431b4617c Mon Sep 17 00:00:00 2001 From: Josh Brewster Date: Thu, 16 Apr 2020 22:50:29 +0200 Subject: [PATCH] libavcodec/libx264: fix reference frame computation based on level The current implementation allows passing levels to libavcodec as integers (such as "31" instead of "3.1"). However, in this case, the maximum reference frame value per level was ignored because libavcodec converted the string to 310 instead of 31. Since libx264 has correctly parsed the level to int (x4->params.i_level_idc), we should rely on this value instead of attempting to parse the level string on our own. Signed-off-by: Josh Brewster --- libavcodec/libx264.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index a08fe0ce76..a4caa5b5db 100644 --- a/libavcodec/libx264.c +++ b/libavcodec/libx264.c @@ -692,25 +692,13 @@ FF_ENABLE_DEPRECATION_WARNINGS x4->params.rc.f_qcompress = avctx->qcompress; /* 0.0 => cbr, 1.0 => constant qp */ if (avctx->refs >= 0) x4->params.i_frame_reference = avctx->refs; - else if (x4->level) { + else if (x4->params.i_level_idc >= 0) { int i; int mbn = AV_CEIL_RSHIFT(avctx->width, 4) * AV_CEIL_RSHIFT(avctx->height, 4); - int level_id = -1; - char *tail; int scale = X264_BUILD < 129 ? 384 : 1; - if (!strcmp(x4->level, "1b")) { - level_id = 9; - } else if (strlen(x4->level) <= 3){ - level_id = av_strtod(x4->level, &tail) * 10 + 0.5; - if (*tail) - level_id = -1; - } - if (level_id <= 0) - av_log(avctx, AV_LOG_WARNING, "Failed to parse level\n"); - for (i = 0; iparams.i_level_idc) x4->params.i_frame_reference = av_clip(x264_levels[i].dpb / mbn / scale, 1, x4->params.i_frame_reference); } -- 2.26.0