From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E26543FC8 for ; Mon, 27 Sep 2021 11:20:20 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: andrzej.p) with ESMTPSA id C678E1F42393 Subject: Re: [PATCH v5 05/10] media: uapi: Add VP9 stateless decoder controls To: Ezequiel Garcia Cc: linux-media@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-staging@lists.linux.dev, Benjamin Gaignard , Boris Brezillon , Fabio Estevam , Greg Kroah-Hartman , Hans Verkuil , Heiko Stuebner , Jernej Skrabec , Mauro Carvalho Chehab , Nicolas Dufresne , NXP Linux Team , Pengutronix Kernel Team , Philipp Zabel , Sascha Hauer , Shawn Guo , kernel@collabora.com, Ezequiel Garcia , Adrian Ratiu , Daniel Almeida References: <20210922101146.13762-1-andrzej.p@collabora.com> <20210922101146.13762-6-andrzej.p@collabora.com> From: Andrzej Pietrasiewicz Message-ID: Date: Mon, 27 Sep 2021 13:20:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 Precedence: bulk X-Mailing-List: linux-staging@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Hi Ezequiel, Thank you for looking into the patch. W dniu 24.09.2021 o 18:23, Ezequiel Garcia pisze: > Hi Andrzej, > > On Wed, Sep 22, 2021 at 12:11:41PM +0200, Andrzej Pietrasiewicz wrote: >> Add the VP9 stateless decoder controls plus the documentation that goes >> with it. >> >> Signed-off-by: Boris Brezillon >> Co-developed-by: Ezequiel Garcia >> Signed-off-by: Ezequiel Garcia >> Signed-off-by: Adrian Ratiu >> Signed-off-by: Andrzej Pietrasiewicz >> Co-developed-by: Daniel Almeida >> Signed-off-by: Daniel Almeida >> --- > [snip] >> + >> +#define V4L2_VP9_FRAME_FLAG_KEY_FRAME 0x001 >> +#define V4L2_VP9_FRAME_FLAG_SHOW_FRAME 0x002 >> +#define V4L2_VP9_FRAME_FLAG_ERROR_RESILIENT 0x004 >> +#define V4L2_VP9_FRAME_FLAG_INTRA_ONLY 0x008 >> +#define V4L2_VP9_FRAME_FLAG_ALLOW_HIGH_PREC_MV 0x010 >> +#define V4L2_VP9_FRAME_FLAG_REFRESH_FRAME_CTX 0x020 >> +#define V4L2_VP9_FRAME_FLAG_PARALLEL_DEC_MODE 0x040 >> +#define V4L2_VP9_FRAME_FLAG_X_SUBSAMPLING 0x080 >> +#define V4L2_VP9_FRAME_FLAG_Y_SUBSAMPLING 0x100 >> +#define V4L2_VP9_FRAME_FLAG_COLOR_RANGE_FULL_SWING 0x200 >> + >> +#define V4L2_VP9_SIGN_BIAS_LAST 0x1 >> +#define V4L2_VP9_SIGN_BIAS_GOLDEN 0x2 >> +#define V4L2_VP9_SIGN_BIAS_ALT 0x4 >> + >> +#define V4L2_VP9_RESET_FRAME_CTX_NONE 0 >> +#define V4L2_VP9_RESET_FRAME_CTX_SPEC 1 >> +#define V4L2_VP9_RESET_FRAME_CTX_ALL 2 >> + >> +#define V4L2_VP9_INTERP_FILTER_EIGHTTAP 0 >> +#define V4L2_VP9_INTERP_FILTER_EIGHTTAP_SMOOTH 1 >> +#define V4L2_VP9_INTERP_FILTER_EIGHTTAP_SHARP 2 >> +#define V4L2_VP9_INTERP_FILTER_BILINEAR 3 >> +#define V4L2_VP9_INTERP_FILTER_SWITCHABLE 4 >> + >> +#define V4L2_VP9_REFERENCE_MODE_SINGLE_REFERENCE 0 >> +#define V4L2_VP9_REFERENCE_MODE_COMPOUND_REFERENCE 1 >> +#define V4L2_VP9_REFERENCE_MODE_SELECT 2 >> + >> +#define V4L2_VP9_PROFILE_MAX 3 >> + >> +#define V4L2_CID_STATELESS_VP9_FRAME (V4L2_CID_CODEC_STATELESS_BASE + 300) >> +/** >> + * struct v4l2_ctrl_vp9_frame - VP9 frame decoding control >> + * >> + * @lf: loop filter parameters. See &v4l2_vp9_loop_filter for more details > > Seems these documentation is missing an ending period for many fields. Thanks for catching punctuation mistakes :) > >> + * @quant: quantization parameters. See &v4l2_vp9_quantization for more details >> + * @seg: segmentation parameters. See &v4l2_vp9_segmentation for more details >> + * @flags: combination of V4L2_VP9_FRAME_FLAG_* flags >> + * @compressed_header_size: compressed header size in bytes >> + * @uncompressed_header_size: uncompressed header size in bytes >> + * @frame_width_minus_1: add 1 to it and you'll get the frame width expressed in pixels >> + * @frame_height_minus_1: add 1 to it and you'll get the frame height expressed in pixels >> + * @render_width_minus_1: add 1 to it and you'll get the expected render width expressed in >> + * pixels. This is not used during the decoding process but might be used by HW scalers >> + * to prepare a frame that's ready for scanout >> + * @render_height_minus_1: add 1 to it and you'll get the expected render height expressed in >> + * pixels. This is not used during the decoding process but might be used by HW scalers >> + * to prepare a frame that's ready for scanout >> + * @last_frame_ts: "last" reference buffer timestamp. >> + * The timestamp refers to the timestamp field in struct v4l2_buffer. >> + * Use v4l2_timeval_to_ns() to convert the struct timeval to a __u64. >> + * @golden_frame_ts: "golden" reference buffer timestamp. >> + * The timestamp refers to the timestamp field in struct v4l2_buffer. >> + * Use v4l2_timeval_to_ns() to convert the struct timeval to a __u64. >> + * @alt_frame_ts: "alt" reference buffer timestamp. >> + * The timestamp refers to the timestamp field in struct v4l2_buffer. >> + * Use v4l2_timeval_to_ns() to convert the struct timeval to a __u64. >> + * @ref_frame_sign_bias: a bitfield specifying whether the sign bias is set for a given >> + * reference frame. Either of V4L2_VP9_SIGN_BIAS_*. >> + * @reset_frame_context: specifies whether the frame context should be reset to default values. >> + * Either of V4L2_VP9_RESET_FRAME_CTX_*. >> + * @frame_context_idx: frame context that should be used/updated >> + * @profile: VP9 profile. Can be 0, 1, 2 or 3 >> + * @bit_depth: bits per components. Can be 8, 10 or 12. Note that not all profiles support >> + * 10 and/or 12 bits depths >> + * @interpolation_filter: specifies the filter selection used for performing inter prediction. >> + * Either of V4L2_VP9_INTERP_FILTER_* >> + * @tile_cols_log2: specifies the base 2 logarithm of the width of each tile (where the width >> + * is measured in units of 8x8 blocks). Shall be less than or equal to 6 >> + * @tile_rows_log2: specifies the base 2 logarithm of the height of each tile (where the height >> + * is measured in units of 8x8 blocks) >> + * @reference_mode: specifies the type of inter prediction to be used. See > > See what? :-) > >> + * Either of V4L2_VP9_REFERENCE_MODE_* > > Other controls use V4L2_VP9_REFERENCE_MODE_{}, {} instead of *. > The same applies to all the documentation. Will update in v6. > >> + * @reserved: padding field. Should be zeroed by applications. >> + */ >> +struct v4l2_ctrl_vp9_frame { >> + struct v4l2_vp9_loop_filter lf; >> + struct v4l2_vp9_quantization quant; >> + struct v4l2_vp9_segmentation seg; >> + __u32 flags; >> + __u16 compressed_header_size; >> + __u16 uncompressed_header_size; >> + __u16 frame_width_minus_1; >> + __u16 frame_height_minus_1; >> + __u16 render_width_minus_1; >> + __u16 render_height_minus_1; >> + __u64 last_frame_ts; >> + __u64 golden_frame_ts; >> + __u64 alt_frame_ts; >> + __u8 ref_frame_sign_bias; >> + __u8 reset_frame_context; >> + __u8 frame_context_idx; >> + __u8 profile; >> + __u8 bit_depth; >> + __u8 interpolation_filter; >> + __u8 tile_cols_log2; >> + __u8 tile_rows_log2; >> + __u8 reference_mode; >> + __u8 reserved[7]; >> +}; >> + > > Also, have you checked html and pdf docs and make sure > it looks as you expect? > Yes I did and it looks as I expected. > Thanks, > Ezequiel >