From patchwork Tue Apr 23 08:34:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Andreas_H=C3=A5kon?= X-Patchwork-Id: 12872 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 CE2CD448870 for ; Tue, 23 Apr 2019 11:34:57 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id AF2AC6805C5; Tue, 23 Apr 2019 11:34:57 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-40133.protonmail.ch (mail-40133.protonmail.ch [185.70.40.133]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1B9F9680550 for ; Tue, 23 Apr 2019 11:34:52 +0300 (EEST) Date: Tue, 23 Apr 2019 08:34:45 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=default; t=1556008491; bh=5twI/BAKGpij12lzNvaGROnxXYknPaSI1XR40ICz8fA=; h=Date:To:From:Reply-To:Subject:In-Reply-To:References:Feedback-ID: From; b=WzZnSYfC5frER4Rhlhvcz2Zge+FamHPvwIRYS0e+R0dGCu6OoCP3VyyFFJ0+FLKbR 1VI+pUGi2DDp1iflwEu8pZMjlE4sNVFWVb4xBWzFdvo7WO/gcf7/aGhmrrE1buLJ7F GRgZSndTaXvbSXzseAy3qGGxQEeFAZF75JIuLXLY= To: FFmpeg development discussions and patches From: =?UTF-8?Q?Andreas_H=C3=A5kon?= Message-ID: In-Reply-To: References: <7D9B0A5171224A4A9A6308992BC4469B418A35F0@shsmsx102.ccr.corp.intel.com> <7D9B0A5171224A4A9A6308992BC4469B418A3E0F@shsmsx102.ccr.corp.intel.com> Feedback-ID: Mx8CaiV20jk_fqXDN0fFpg3vRaGkb9VCTrYRnZNHwEija3aOdqvFspzl6ODkmHrlSKJSx29p-LzkuvS_96L02A==:Ext:ProtonMail 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 autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on mail.protonmail.ch Subject: Re: [FFmpeg-devel] [PATCH] libavcodec: QSV protect GPB code with CO3 define 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" ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Tuesday, 23 de April de 2019 9:29, Andreas Håkon wrote: > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ > On Tuesday, 23 de April de 2019 8:39, Li, Zhong zhong.li@intel.com wrote: > > > I belive they are different. If you extend the MARCIO, they are actually: > > IMHO, your patch is only needed when disable "QSV_HAVE_CO3", but tiket#7839 is not root caused now. > > I will consider to accept it when tiket#7839 is root caused. > > Hi Li, > > With all due respect. > You're assuming that: > "QSV_VERSION_ATLEAST(1, 18)" > "QSV_VERSION_ATLEAST(1, 11)" > So, > "QSV_VERSION_ATLEAST(1, 18)" implies that "QSV_VERSION_ATLEAST(1, 11)" > > But the CODE doesn't say that! > > The code uses: > "#if QSV_HAVE_CO3" > > And then every use of the "co3" member needs to be protected by this clausule. > Futhermore, this is true in every part of the source code in "qsvenc.c"... except at > two points (I need to review my patch, as I noticed another block not protected). > > This is a must, even if "QSV_VERSION_ATLEAST(1, 18)" > "QSV_VERSION_ATLEAST(1, 11)" > > Why? > Because the option "QSV_HAVE_CO3" can be rewriten (for example, disabled). > > So the conclusion is that the code doesn't say "QSV_VERSION_ATLEAST(x)". > > Anyway, thanks for discussing it. Stay tuned, while I prepare another patch > protecting all cases. > > Regards. > A.H. > > ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Hi Li, Here the updated patch. I hope you agree with it! Regards. A.H. --- From 211c6704e4e4272221fbc8c908b99307b11e1800 Mon Sep 17 00:00:00 2001 From: Andreas Hakon Date: Tue, 23 Apr 2019 09:25:15 +0100 Subject: [PATCH] libavcodec: QSV protect GPB code with CO3 define [v2] This patch protects some code using the member "co3" inside the qsvenc.c file. The first block is a simple debug message, so it doesn't generates troubles. The second block is an initialization using of the "extco3" for the QVBR mode. After this patch it's safe to enable/disable some options in the source code without triggering compilation errors. Signed-off-by: Andreas Hakon --- libavcodec/qsvenc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index a03ab69..feedceb 100644 --- a/libavcodec/qsvenc.c +++ b/libavcodec/qsvenc.c @@ -267,10 +267,12 @@ static void dump_video_param(AVCodecContext *avctx, QSVEncContext *q, #endif #endif +#if QSV_HAVE_CO3 #if QSV_HAVE_GPB if (avctx->codec_id == AV_CODEC_ID_HEVC) av_log(avctx, AV_LOG_VERBOSE,"GPB: %s\n", print_threestate(co3->GPB)); #endif +#endif if (avctx->codec_id == AV_CODEC_ID_H264) { av_log(avctx, AV_LOG_VERBOSE, "Entropy coding: %s; MaxDecFrameBuffering: %"PRIu16"\n", @@ -598,9 +600,11 @@ static int init_video_param(AVCodecContext *avctx, QSVEncContext *q) q->param.mfx.MaxKbps = max_bitrate_kbps / brc_param_multiplier; q->param.mfx.BRCParamMultiplier = brc_param_multiplier; #if QSV_HAVE_QVBR +#if QSV_HAVE_CO3 if (q->param.mfx.RateControlMethod == MFX_RATECONTROL_QVBR) q->extco3.QVBRQuality = av_clip(avctx->global_quality, 0, 51); #endif +#endif break; case MFX_RATECONTROL_CQP: quant = avctx->global_quality / FF_QP2LAMBDA;