From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8C470C433F5 for ; Wed, 5 Jan 2022 12:28:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=FuMC/yDqiRQ45sE4fhHIfvPR3yZC6dN/lXfqIlfgfvg=; b=JahIa4rWl0uCpW 0PJHTZcaPAF4a2ZSfpd1Xe7CgvTmshfKw1GEP6G25YrQJjikITOSS1lGWGxBY2jzpB6XynM1bvnhM wOmWHIpTqsFqhZ71+6Aac8juUu0x+1CyQn52s6iGW75P6dhAZ6iAtBhBYAm19GwlWRZusr+XJ2vSa bFgIeV+QW3XtHGxUfCFisgZ9EamiYTIODHwjFKckXBb+Gb5BEsgAHckRp0i4cJmYGGmG83QDXg6Xm UUyBWRoqfNZzXewebGGcR0Ejh5flr6UykvBWVkTslGLZovP+pPdd2aVxe+bsLclZwzYTu3WdKPnVc ALqjR4Wkitv/lu/AOPKA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n55NP-00ElTT-Ec; Wed, 05 Jan 2022 12:26:47 +0000 Received: from mail-qv1-xf31.google.com ([2607:f8b0:4864:20::f31]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1n55NL-00ElS7-Qs for linux-arm-kernel@lists.infradead.org; Wed, 05 Jan 2022 12:26:45 +0000 Received: by mail-qv1-xf31.google.com with SMTP id ke6so37387372qvb.1 for ; Wed, 05 Jan 2022 04:26:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vanguardiasur-com-ar.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=+6zUFKikZ5uI7X+WBRedpvpkwDXnTgkeRYffGhJplBQ=; b=v0GeznWOJp1intYhv38FsyRKC2LY+s7ndDGtu8347+i9nbXXv04HckdcZQOYkPKp90 /jX/cn0VqKe2GUgFg8xH4/RRsX2gOU2vi4YIq2jnAXgtJFu8NKWOmv5UTm7JKOBG3wVd yX9NM8tByP/uwWU3/WPQ7QTsUzFb47l90lqhxnFgpjcN80pQu3wHho0EGORmw4jD4Hze 2N7+JCrk4rGAxSTsjSt08zwMWj9DQKe79mw7iOhhxX8p6AUqdjfORxMRC/f4QgTqsNJB wYnJmPOreCRXYJ/LqiEG5E0tR+gwvKZVim6Fcfba4VyFEW0Zya6GV1LFeZO/aFjf05Op DDbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=+6zUFKikZ5uI7X+WBRedpvpkwDXnTgkeRYffGhJplBQ=; b=0sghHhUlw+GR052+8ZS8JvOwzGz8wbSuL3M+8NmbvSvkpkbef3G32BNCF0Z574Qh2M 5cmJcMYZt1DGmFD4TV++TEnCvDUhvLDwde7WprjcfRno1OXZ8pPHhzDiIzLEVYUdMmmi 2eM41nPo6c4BkBzcVKwA+JWoiEZUoYnh3BQFmcLa+t1xIjIN5Ku4lgmmrMKVxVZ73FRx EGjMT2lMSyQdPQ2pNGDhDiJ3KR6U3gt+d1vJgZjmcWZEtloSOinSDJWkhf4N9y2oV5Sc Cd4a2N5qOiZhDz8WqgpOf8kMxfptZFkUrG2Ck6XEBjlbJ+nlXmqT/PPuh3KSo7nxzMNK HxiQ== X-Gm-Message-State: AOAM533BqvPpn+SHoBfzODaHltvKRFLrxGicpSc7YAxOfcmT9MsmI0WK V3UifO561vcAAj3OpKptLLlr2JymmkXHoQ== X-Google-Smtp-Source: ABdhPJxqbVB7XWk6LPZe9NdWapRfu4IObMFem/esLdwpybt5KuvKvaxSKJK/v6LXDUp/1xX65da8KA== X-Received: by 2002:a05:6214:262d:: with SMTP id gv13mr49591568qvb.23.1641385601790; Wed, 05 Jan 2022 04:26:41 -0800 (PST) Received: from eze-laptop ([186.122.18.30]) by smtp.gmail.com with ESMTPSA id a15sm36481925qtb.5.2022.01.05.04.26.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Jan 2022 04:26:39 -0800 (PST) Date: Wed, 5 Jan 2022 09:26:33 -0300 From: Ezequiel Garcia To: Benjamin Gaignard Cc: mchehab@kernel.org, p.zabel@pengutronix.de, gregkh@linuxfoundation.org, hverkuil-cisco@xs4all.nl, jernej.skrabec@gmail.com, nicolas.dufresne@collabora.co.uk, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-staging@lists.linux.dev, kernel@collabora.com Subject: Re: [PATCH v4 1/2] media: hevc: Remove RPS named flags Message-ID: References: <20220104073842.1791639-1-benjamin.gaignard@collabora.com> <20220104073842.1791639-2-benjamin.gaignard@collabora.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220104073842.1791639-2-benjamin.gaignard@collabora.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220105_042644_027525_8D8ADEC2 X-CRM114-Status: GOOD ( 26.60 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Benjamin, On Tue, Jan 04, 2022 at 08:38:41AM +0100, Benjamin Gaignard wrote: > Marking a picture as long-term reference is valid for DPB but not for RPS. > Change flag name to match with it description in HEVC spec chapiter > "8.3.2 Decoding process for reference picture set". > Remove the other unused RPS flags. > A change like this, which is affecting a kernel interface and has impact on userspace and drivers, requires a lot more attention in the commit description. If I understand correctly, this change is related to how HEVC was first introduced and how the DPB was originally represented in V4L2. Originally, a DPB entry struct v4l2_hevc_dpb_entry had an rps field which can be: V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_BEFORE V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_AFTER V4L2_HEVC_DPB_ENTRY_RPS_LT_CURR Perhaps this idea followed libva, where a VAPictureHEVC has flags field which can be: VA_PICTURE_HEVC_RPS_ST_CURR_BEFORE, VA_PICTURE_HEVC_RPS_ST_CURR_AFTER, VA_PICTURE_HEVC_RPS_LT_CURR, VA_PICTURE_HEVC_LONG_TERM_REFERENCE In this representation, the sets PocStCurrBefore, PocStCurrAfter, PocLtCurr are implicit, and must be built by the kernel from the DPB entries struct v4l2_hevc_dpb_entry, using the information in the rps field. Last year, we changed this with your commit: commit d395a78db9eabd12633b39e05c80e803543b6590 Author: Benjamin Gaignard Date: Thu Jun 3 13:49:57 2021 +0200 media: hevc: Add decode params control Which added the control v4l2_ctrl_hevc_decode_params, which includes the sets PocStCurrBefore, PocStCurrAfter, PocLtCurr explicitly and therefore moved the responsability of creating and maintaining the sets to userspace. This effectively made the rps field in the struct v4l2_hevc_dpb_entry useless. The longterm flag is still needed though for a DPB entry. With this in mind, you could even say this commit is doing actually two things: 1. Removes the unused rps field. 2. Adds a flag field, for the longterm DPB entry boolean. Do you think a single byte of flags will be OK for a DPB entry? I think so, but I'd love yours/others input here. If the above is more or less accurate then, and provided you submit a new version with a more detailed commit description: Reviewed-by: Ezequiel Garcia Also, a small question: > Signed-off-by: Benjamin Gaignard > --- > version 4: > - check flags with & and not == > > Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 8 +++----- > drivers/staging/media/hantro/hantro_g2_hevc_dec.c | 2 +- > drivers/staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- > include/media/hevc-ctrls.h | 6 ++---- > 4 files changed, 7 insertions(+), 11 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > index e141f0e4eec9..38da33e61c3d 100644 > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > @@ -3166,11 +3166,9 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > :c:func:`v4l2_timeval_to_ns()` function to convert the struct > :c:type:`timeval` in struct :c:type:`v4l2_buffer` to a __u64. > * - __u8 > - - ``rps`` > - - The reference set for the reference frame > - (V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_BEFORE, > - V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_AFTER or > - V4L2_HEVC_DPB_ENTRY_RPS_LT_CURR) > + - ``flags`` > + - Long term flag for the reference frame > + (V4L2_HEVC_DPB_ENTRY_LONG_TERM_REFERENCE) Is this longterm flag associated in any way to a syntax element or an HEVC process? If so, please document that. Thanks, Ezequiel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel