Messages in this thread | | | From | Jernej Škrabec <> | Subject | Re: [linux-sunxi] [PATCH v3 2/2] media: cedrus: Add H264 decoding support | Date | Tue, 12 Feb 2019 18:46:38 +0100 |
| |
Dne torek, 12. februar 2019 ob 11:43:14 CET je Maxime Ripard napisal(a): > Hi, > > On Mon, Feb 11, 2019 at 08:21:31PM +0100, Jernej Škrabec wrote: > > > + reg = 0; > > > + /* > > > + * FIXME: This bit tells the video engine to use the default > > > + * quantization matrices. This will obviously need to be > > > + * changed to support the profiles supporting custom > > > + * quantization matrices. > > > + */ > > > + reg |= VE_H264_SHS_QP_SCALING_MATRIX_DEFAULT; > > > > This flag should not be needed anymore. From what I see, you correctly set > > scaling matrix every time. > > The scaling matrix control is optional, so I guess we should protect > that by a check on whether that control has been set or not. What do > you think?
Is it? Consider following snippet from your patch:
<snip> { .id = V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX, .elem_size = sizeof(struct v4l2_ctrl_h264_scaling_matrix), .codec = CEDRUS_CODEC_H264, .required = true, }, <snip>
Doesn't "required = true" mean that it's mandatory? But yes, if it's optional, then you should add a check.
Best regards, Jernej
| |