On 03/04/2021 20:07, Ezequiel Garcia wrote: > As stated in the MPEG-2 specification, section 6.3.7 "Quant matrix > extension": > > Each quantisation matrix has a default set of values. When a > sequence_header_code is decoded all matrices shall be reset to > their default values. User defined matrices may be downloaded > and this can occur in a sequence_header() or in a > quant_matrix_extension(). > > The load_intra_quantiser_matrix syntax elements are transmitted > in the bistream headers, signalling that a quantisation matrix
bistream -> bitstream > needs to be loaded and used for pictures transmitted afterwards > (until the matrices are reset). > > This "load" semantics are implemented in the V4L2 interface > without the need of any "load" flags: passing the control > is effectively a load. > > Therefore, rework the V4L2_CID_MPEG_VIDEO_MPEG2_QUANTISATION > semantics to match the MPEG-2 semantics. Quantisation matrices > values are now initialized by the V4L2 control core to their > reset default value, and applications are expected to reset > their values as specified. > > The quantisation control is therefore optional, and used to > load bitstream-defined values in the quantisation matrices. > > Signed-off-by: Ezequiel Garcia <[email protected]> > Co-developed-by: Nicolas Dufresne <[email protected]> > Signed-off-by: Nicolas Dufresne <[email protected]> > --- > .../media/v4l/ext-ctrls-codec.rst | 17 --------- > drivers/media/v4l2-core/v4l2-ctrls.c | 21 ++++++++++ > .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 38 +------------------ > include/media/mpeg2-ctrls.h | 5 --- > 4 files changed, 23 insertions(+), 58 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > index 151d1c676b6e..d9546f0aa2e8 100644 > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > @@ -1746,23 +1746,6 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - > :stub-columns: 0 > :widths: 1 1 2 > > - * - __u8 > - - ``load_intra_quantiser_matrix`` > - - One bit to indicate whether to load the ``intra_quantiser_matrix`` > data. > - * - __u8 > - - ``load_non_intra_quantiser_matrix`` > - - One bit to indicate whether to load the > ``non_intra_quantiser_matrix`` > - data. > - * - __u8 > - - ``load_chroma_intra_quantiser_matrix`` > - - One bit to indicate whether to load the > - ``chroma_intra_quantiser_matrix`` data, only relevant for non-4:2:0 YUV > - formats. > - * - __u8 > - - ``load_chroma_non_intra_quantiser_matrix`` > - - One bit to indicate whether to load the > - ``chroma_non_intra_quantiser_matrix`` data, only relevant for non-4:2:0 > - YUV formats. > * - __u8 > - ``intra_quantiser_matrix[64]`` > - The quantisation matrix coefficients for intra-coded frames, in > zigzag > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c > b/drivers/media/v4l2-core/v4l2-ctrls.c > index 74f5ca1a5f6c..5d92a2b33a6e 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -57,6 +57,18 @@ static bool is_new_manual(const struct v4l2_ctrl *master) > return master->is_auto && master->val == master->manual_mode_value; > } > > +/* Default MPEG-2 quantisation coefficients, from the specification. */ Default -> Default intra > +static const u8 mpeg2_intra_quant_matrix[64] = { > + 8, 16, 16, 19, 16, 19, 22, 22, > + 22, 22, 22, 22, 26, 24, 26, 27, > + 27, 27, 26, 26, 26, 26, 27, 27, > + 27, 29, 29, 29, 34, 34, 34, 29, > + 29, 29, 27, 27, 29, 29, 32, 32, > + 34, 34, 37, 38, 37, 35, 35, 34, > + 35, 38, 38, 40, 40, 40, 48, 48, > + 46, 46, 56, 56, 58, 69, 69, 83 > +}; > + > /* Returns NULL or a character pointer array containing the menu for > the given control ID. The pointer array ends with a NULL pointer. > An empty string signifies a menu entry that is invalid. This allows > @@ -1656,6 +1668,7 @@ static void std_init_compound(const struct v4l2_ctrl > *ctrl, u32 idx, > union v4l2_ctrl_ptr ptr) > { > struct v4l2_ctrl_mpeg2_slice_params *p_mpeg2_slice_params; > + struct v4l2_ctrl_mpeg2_quantisation *p_mpeg2_quant; > struct v4l2_ctrl_vp8_frame *p_vp8_frame; > struct v4l2_ctrl_fwht_params *p_fwht_params; > void *p = ptr.p + idx * ctrl->elem_size; > @@ -1680,6 +1693,14 @@ static void std_init_compound(const struct v4l2_ctrl > *ctrl, u32 idx, > p_mpeg2_slice_params->picture.picture_coding_type = > V4L2_MPEG2_PICTURE_CODING_TYPE_I; > break; > + case V4L2_CTRL_TYPE_MPEG2_QUANTISATION: > + p_mpeg2_quant = p; > + > + memcpy(p_mpeg2_quant->intra_quantiser_matrix, > + mpeg2_intra_quant_matrix, > + ARRAY_SIZE(mpeg2_intra_quant_matrix)); > + memset(p_mpeg2_quant->non_intra_quantiser_matrix, 16, 64); 64 -> sizeof(p_mpeg2_quant->non_intra_quantiser_matrix) Also add a comment before the memset: /* * The default non-intra MPEG-2 quantisation coefficients are all 16, * as per the specification. */ > + break; > case V4L2_CTRL_TYPE_VP8_FRAME: > p_vp8_frame = p; > p_vp8_frame->num_dct_parts = 1; > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > index 459f71679a4f..e3154f631858 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > @@ -13,30 +13,6 @@ > #include "cedrus_hw.h" > #include "cedrus_regs.h" > > -/* Default MPEG-2 quantisation coefficients, from the specification. */ > - > -static const u8 intra_quantisation_matrix_default[64] = { > - 8, 16, 16, 19, 16, 19, 22, 22, > - 22, 22, 22, 22, 26, 24, 26, 27, > - 27, 27, 26, 26, 26, 26, 27, 27, > - 27, 29, 29, 29, 34, 34, 34, 29, > - 29, 29, 27, 27, 29, 29, 32, 32, > - 34, 34, 37, 38, 37, 35, 35, 34, > - 35, 38, 38, 40, 40, 40, 48, 48, > - 46, 46, 56, 56, 58, 69, 69, 83 > -}; > - > -static const u8 non_intra_quantisation_matrix_default[64] = { > - 16, 16, 16, 16, 16, 16, 16, 16, > - 16, 16, 16, 16, 16, 16, 16, 16, > - 16, 16, 16, 16, 16, 16, 16, 16, > - 16, 16, 16, 16, 16, 16, 16, 16, > - 16, 16, 16, 16, 16, 16, 16, 16, > - 16, 16, 16, 16, 16, 16, 16, 16, > - 16, 16, 16, 16, 16, 16, 16, 16, > - 16, 16, 16, 16, 16, 16, 16, 16 > -}; > - > static enum cedrus_irq_status cedrus_mpeg2_irq_status(struct cedrus_ctx *ctx) > { > struct cedrus_dev *dev = ctx->dev; > @@ -99,12 +75,7 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, > struct cedrus_run *run) > cedrus_engine_enable(ctx, CEDRUS_CODEC_MPEG2); > > /* Set intra quantisation matrix. */ > - > - if (quantisation && quantisation->load_intra_quantiser_matrix) > - matrix = quantisation->intra_quantiser_matrix; > - else > - matrix = intra_quantisation_matrix_default; > - > + matrix = quantisation->intra_quantiser_matrix; > for (i = 0; i < 64; i++) { > reg = VE_DEC_MPEG_IQMINPUT_WEIGHT(i, matrix[i]); > reg |= VE_DEC_MPEG_IQMINPUT_FLAG_INTRA; > @@ -113,12 +84,7 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, > struct cedrus_run *run) > } > > /* Set non-intra quantisation matrix. */ > - > - if (quantisation && quantisation->load_non_intra_quantiser_matrix) > - matrix = quantisation->non_intra_quantiser_matrix; > - else > - matrix = non_intra_quantisation_matrix_default; > - > + matrix = quantisation->non_intra_quantiser_matrix; > for (i = 0; i < 64; i++) { > reg = VE_DEC_MPEG_IQMINPUT_WEIGHT(i, matrix[i]); > reg |= VE_DEC_MPEG_IQMINPUT_FLAG_NON_INTRA; > diff --git a/include/media/mpeg2-ctrls.h b/include/media/mpeg2-ctrls.h > index b8adf3ac2c1d..8ea2c7f3a172 100644 > --- a/include/media/mpeg2-ctrls.h > +++ b/include/media/mpeg2-ctrls.h > @@ -68,11 +68,6 @@ struct v4l2_ctrl_mpeg2_slice_params { > > struct v4l2_ctrl_mpeg2_quantisation { > /* ISO/IEC 13818-2, ITU-T Rec. H.262: Quant matrix extension */ > - __u8 load_intra_quantiser_matrix; > - __u8 load_non_intra_quantiser_matrix; > - __u8 load_chroma_intra_quantiser_matrix; > - __u8 load_chroma_non_intra_quantiser_matrix; > - > __u8 intra_quantiser_matrix[64]; > __u8 non_intra_quantiser_matrix[64]; > __u8 chroma_intra_quantiser_matrix[64]; > Regards, Hans
