* [PATCH v1 0/2] Intra-refresh type control @ 2022-01-24 10:11 Dikshita Agarwal 2022-01-24 10:11 ` [PATCH v1 1/2] media: v4l2-ctrls: Add intra-refresh " Dikshita Agarwal 2022-01-24 10:11 ` [PATCH v1 2/2] venus: venc: Add support for intra-refresh mode Dikshita Agarwal 0 siblings, 2 replies; 7+ messages in thread From: Dikshita Agarwal @ 2022-01-24 10:11 UTC (permalink / raw) To: linux-media, linux-kernel, linux-arm-msm Cc: hverkuil-cisco, ezequiel, vgarodia, stanimir.varbanov, Dikshita Agarwal Hi, This series add a new intra-refresh type control for encoders. this can be used to specify which intra refresh to be enabled, random, cyclic or none. Change since v0: Dropped INTRA_REFRESH_TYPE_NONE as it was not needed. Intra refresh period value as zero will disable the intra refresh. Thanks, Dikshita Dikshita Agarwal (2): media: v4l2-ctrls: Add intra-refresh type control venus: venc: Add support for intra-refresh mode .../userspace-api/media/v4l/ext-ctrls-codec.rst | 23 ++++++++++++++++++++++ drivers/media/platform/qcom/venus/core.h | 1 + drivers/media/platform/qcom/venus/venc.c | 3 ++- drivers/media/platform/qcom/venus/venc_ctrls.c | 8 ++++++++ drivers/media/v4l2-core/v4l2-ctrls-defs.c | 9 +++++++++ include/uapi/linux/v4l2-controls.h | 5 +++++ 6 files changed, 48 insertions(+), 1 deletion(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 1/2] media: v4l2-ctrls: Add intra-refresh type control 2022-01-24 10:11 [PATCH v1 0/2] Intra-refresh type control Dikshita Agarwal @ 2022-01-24 10:11 ` Dikshita Agarwal 2022-01-25 20:10 ` Nicolas Dufresne 2022-02-15 8:21 ` Hans Verkuil 2022-01-24 10:11 ` [PATCH v1 2/2] venus: venc: Add support for intra-refresh mode Dikshita Agarwal 1 sibling, 2 replies; 7+ messages in thread From: Dikshita Agarwal @ 2022-01-24 10:11 UTC (permalink / raw) To: linux-media, linux-kernel, linux-arm-msm Cc: hverkuil-cisco, ezequiel, vgarodia, stanimir.varbanov, Dikshita Agarwal From: Dikshita Agarwal <quic_dikshita@quicinc.com> Add a control to set intra-refresh type. Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> --- .../userspace-api/media/v4l/ext-ctrls-codec.rst | 23 ++++++++++++++++++++++ drivers/media/v4l2-core/v4l2-ctrls-defs.c | 9 +++++++++ include/uapi/linux/v4l2-controls.h | 5 +++++ 3 files changed, 37 insertions(+) diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst index e141f0e..54b42e1 100644 --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst @@ -1180,6 +1180,29 @@ enum v4l2_mpeg_video_h264_entropy_mode - is set to non zero value. Applicable to H264, H263 and MPEG4 encoder. +``V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE (enum)`` + +enum v4l2_mpeg_video_intra_refresh_type - + Sets the type of intra refresh. The period to refresh + the whole frame is specified by V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD. + Note if the client sets this control to either ``V4L2_MPEG_VIDEO_INTRA_REFRESH_RANDOM`` + or ``V4L2_MPEG_VIDEO_INTRA_REFRESH_CYCLIC`` the ``V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB`` + control shall be ignored. + Applicable to H264, H263 and MPEG4 encoder. Possible values are: + +.. tabularcolumns:: |p{9.6cm}|p{7.9cm}| + +.. flat-table:: + :header-rows: 0 + :stub-columns: 0 + + * - ``V4L2_MPEG_VIDEO_INTRA_REFRESH_RANDOM`` + - The whole frame is completely refreshed randomly + after the specified period. + * - ``V4L2_MPEG_VIDEO_INTRA_REFRESH_CYCLIC`` + - The whole frame MBs are completely refreshed in cyclic order + after the specified period. + ``V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD (integer)`` Intra macroblock refresh period. This sets the period to refresh the whole frame. In other words, this defines the number of frames diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c index 54ca4e6..f13f587 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c @@ -572,6 +572,11 @@ const char * const *v4l2_ctrl_get_menu(u32 id) "VBV/CPB Limit", NULL, }; + static const char * const intra_refresh_type[] = { + "Random", + "Cyclic", + NULL, + }; switch (id) { case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ: @@ -705,6 +710,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id) return hevc_start_code; case V4L2_CID_CAMERA_ORIENTATION: return camera_orientation; + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: + return intra_refresh_type; default: return NULL; } @@ -834,6 +841,7 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE: return "Decoder Slice Interface"; case V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER: return "MPEG4 Loop Filter Enable"; case V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB: return "Number of Intra Refresh MBs"; + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: return "Intra Refresh Type"; case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD: return "Intra Refresh Period"; case V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE: return "Frame Level Rate Control Enable"; case V4L2_CID_MPEG_VIDEO_MB_RC_ENABLE: return "H264 MB Level Rate Control"; @@ -1360,6 +1368,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, case V4L2_CID_STATELESS_H264_DECODE_MODE: case V4L2_CID_STATELESS_H264_START_CODE: case V4L2_CID_CAMERA_ORIENTATION: + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: *type = V4L2_CTRL_TYPE_MENU; break; case V4L2_CID_LINK_FREQ: diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h index c8e0f84..9650b71 100644 --- a/include/uapi/linux/v4l2-controls.h +++ b/include/uapi/linux/v4l2-controls.h @@ -443,6 +443,11 @@ enum v4l2_mpeg_video_multi_slice_mode { #define V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES (V4L2_CID_CODEC_BASE+234) #define V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (V4L2_CID_CODEC_BASE+235) #define V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD (V4L2_CID_CODEC_BASE+236) +#define V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE (V4L2_CID_CODEC_BASE+237) +enum v4l2_mpeg_video_intra_refresh_type { + V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_RANDOM = 0, + V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_CYCLIC = 1, +}; /* CIDs for the MPEG-2 Part 2 (H.262) codec */ #define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL (V4L2_CID_CODEC_BASE+270) -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] media: v4l2-ctrls: Add intra-refresh type control 2022-01-24 10:11 ` [PATCH v1 1/2] media: v4l2-ctrls: Add intra-refresh " Dikshita Agarwal @ 2022-01-25 20:10 ` Nicolas Dufresne 2022-02-15 8:21 ` Hans Verkuil 1 sibling, 0 replies; 7+ messages in thread From: Nicolas Dufresne @ 2022-01-25 20:10 UTC (permalink / raw) To: Dikshita Agarwal, linux-media, linux-kernel, linux-arm-msm Cc: hverkuil-cisco, ezequiel, vgarodia, stanimir.varbanov, Dikshita Agarwal Le lundi 24 janvier 2022 à 15:41 +0530, Dikshita Agarwal a écrit : > From: Dikshita Agarwal <quic_dikshita@quicinc.com> > > Add a control to set intra-refresh type. > > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > --- > .../userspace-api/media/v4l/ext-ctrls-codec.rst | 23 ++++++++++++++++++++++ > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 9 +++++++++ > include/uapi/linux/v4l2-controls.h | 5 +++++ > 3 files changed, 37 insertions(+) > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > index e141f0e..54b42e1 100644 > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > @@ -1180,6 +1180,29 @@ enum v4l2_mpeg_video_h264_entropy_mode - > is set to non zero value. > Applicable to H264, H263 and MPEG4 encoder. > > +``V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE (enum)`` > + > +enum v4l2_mpeg_video_intra_refresh_type - > + Sets the type of intra refresh. The period to refresh > + the whole frame is specified by V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD. > + Note if the client sets this control to either ``V4L2_MPEG_VIDEO_INTRA_REFRESH_RANDOM`` > + or ``V4L2_MPEG_VIDEO_INTRA_REFRESH_CYCLIC`` the ``V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB`` > + control shall be ignored. > + Applicable to H264, H263 and MPEG4 encoder. Possible values are: > + > +.. tabularcolumns:: |p{9.6cm}|p{7.9cm}| > + > +.. flat-table:: > + :header-rows: 0 > + :stub-columns: 0 > + > + * - ``V4L2_MPEG_VIDEO_INTRA_REFRESH_RANDOM`` > + - The whole frame is completely refreshed randomly > + after the specified period. > + * - ``V4L2_MPEG_VIDEO_INTRA_REFRESH_CYCLIC`` > + - The whole frame MBs are completely refreshed in cyclic order > + after the specified period. > + > ``V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD (integer)`` > Intra macroblock refresh period. This sets the period to refresh > the whole frame. In other words, this defines the number of frames > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c > index 54ca4e6..f13f587 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c > @@ -572,6 +572,11 @@ const char * const *v4l2_ctrl_get_menu(u32 id) > "VBV/CPB Limit", > NULL, > }; > + static const char * const intra_refresh_type[] = { > + "Random", > + "Cyclic", > + NULL, > + }; > > switch (id) { > case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ: > @@ -705,6 +710,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id) > return hevc_start_code; > case V4L2_CID_CAMERA_ORIENTATION: > return camera_orientation; > + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: > + return intra_refresh_type; > default: > return NULL; > } > @@ -834,6 +841,7 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE: return "Decoder Slice Interface"; > case V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER: return "MPEG4 Loop Filter Enable"; > case V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB: return "Number of Intra Refresh MBs"; > + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: return "Intra Refresh Type"; > case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD: return "Intra Refresh Period"; > case V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE: return "Frame Level Rate Control Enable"; > case V4L2_CID_MPEG_VIDEO_MB_RC_ENABLE: return "H264 MB Level Rate Control"; > @@ -1360,6 +1368,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, > case V4L2_CID_STATELESS_H264_DECODE_MODE: > case V4L2_CID_STATELESS_H264_START_CODE: > case V4L2_CID_CAMERA_ORIENTATION: > + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: > *type = V4L2_CTRL_TYPE_MENU; > break; > case V4L2_CID_LINK_FREQ: > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h > index c8e0f84..9650b71 100644 > --- a/include/uapi/linux/v4l2-controls.h > +++ b/include/uapi/linux/v4l2-controls.h > @@ -443,6 +443,11 @@ enum v4l2_mpeg_video_multi_slice_mode { > #define V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES (V4L2_CID_CODEC_BASE+234) > #define V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (V4L2_CID_CODEC_BASE+235) > #define V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD (V4L2_CID_CODEC_BASE+236) > +#define V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE (V4L2_CID_CODEC_BASE+237) > +enum v4l2_mpeg_video_intra_refresh_type { > + V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_RANDOM = 0, > + V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_CYCLIC = 1, > +}; > > /* CIDs for the MPEG-2 Part 2 (H.262) codec */ > #define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL (V4L2_CID_CODEC_BASE+270) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] media: v4l2-ctrls: Add intra-refresh type control 2022-01-24 10:11 ` [PATCH v1 1/2] media: v4l2-ctrls: Add intra-refresh " Dikshita Agarwal 2022-01-25 20:10 ` Nicolas Dufresne @ 2022-02-15 8:21 ` Hans Verkuil 2022-02-21 6:02 ` dikshita 1 sibling, 1 reply; 7+ messages in thread From: Hans Verkuil @ 2022-02-15 8:21 UTC (permalink / raw) To: Dikshita Agarwal, linux-media, linux-kernel, linux-arm-msm Cc: ezequiel, vgarodia, stanimir.varbanov, Dikshita Agarwal Hi Dikshita, Some comments below: On 1/24/22 11:11, Dikshita Agarwal wrote: > From: Dikshita Agarwal <quic_dikshita@quicinc.com> > > Add a control to set intra-refresh type. > > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > --- > .../userspace-api/media/v4l/ext-ctrls-codec.rst | 23 ++++++++++++++++++++++ > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 9 +++++++++ > include/uapi/linux/v4l2-controls.h | 5 +++++ > 3 files changed, 37 insertions(+) > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > index e141f0e..54b42e1 100644 > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > @@ -1180,6 +1180,29 @@ enum v4l2_mpeg_video_h264_entropy_mode - > is set to non zero value. > Applicable to H264, H263 and MPEG4 encoder. > > +``V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE (enum)`` > + > +enum v4l2_mpeg_video_intra_refresh_type - > + Sets the type of intra refresh. The period to refresh > + the whole frame is specified by V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD. > + Note if the client sets this control to either ``V4L2_MPEG_VIDEO_INTRA_REFRESH_RANDOM`` > + or ``V4L2_MPEG_VIDEO_INTRA_REFRESH_CYCLIC`` the ``V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB`` > + control shall be ignored. Since this control has only two possible values, that would mean that, if this control is present, then REFRESH_MB is always ignored. It seems to me that you need a third option here that specifically selects the REFRESH_MB method. Also, this needs to be documented as well in REFRESH_MB (i.e. it is ignored if this TYPE control is present and is set to something other than REFRESH_MB). > + Applicable to H264, H263 and MPEG4 encoder. Possible values are: > + > +.. tabularcolumns:: |p{9.6cm}|p{7.9cm}| > + > +.. flat-table:: > + :header-rows: 0 > + :stub-columns: 0 > + > + * - ``V4L2_MPEG_VIDEO_INTRA_REFRESH_RANDOM`` I think you should add _TYPE after REFRESH in these names to clearly specify that this is setting the refresh *type*. > + - The whole frame is completely refreshed randomly > + after the specified period. > + * - ``V4L2_MPEG_VIDEO_INTRA_REFRESH_CYCLIC`` > + - The whole frame MBs are completely refreshed in cyclic order > + after the specified period. > + > ``V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD (integer)`` > Intra macroblock refresh period. This sets the period to refresh > the whole frame. In other words, this defines the number of frames > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c > index 54ca4e6..f13f587 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c > @@ -572,6 +572,11 @@ const char * const *v4l2_ctrl_get_menu(u32 id) > "VBV/CPB Limit", > NULL, > }; > + static const char * const intra_refresh_type[] = { > + "Random", > + "Cyclic", > + NULL, > + }; > > switch (id) { > case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ: > @@ -705,6 +710,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id) > return hevc_start_code; > case V4L2_CID_CAMERA_ORIENTATION: > return camera_orientation; > + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: > + return intra_refresh_type; > default: > return NULL; > } > @@ -834,6 +841,7 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE: return "Decoder Slice Interface"; > case V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER: return "MPEG4 Loop Filter Enable"; > case V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB: return "Number of Intra Refresh MBs"; > + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: return "Intra Refresh Type"; > case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD: return "Intra Refresh Period"; > case V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE: return "Frame Level Rate Control Enable"; > case V4L2_CID_MPEG_VIDEO_MB_RC_ENABLE: return "H264 MB Level Rate Control"; > @@ -1360,6 +1368,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, > case V4L2_CID_STATELESS_H264_DECODE_MODE: > case V4L2_CID_STATELESS_H264_START_CODE: > case V4L2_CID_CAMERA_ORIENTATION: > + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: > *type = V4L2_CTRL_TYPE_MENU; > break; > case V4L2_CID_LINK_FREQ: > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h > index c8e0f84..9650b71 100644 > --- a/include/uapi/linux/v4l2-controls.h > +++ b/include/uapi/linux/v4l2-controls.h > @@ -443,6 +443,11 @@ enum v4l2_mpeg_video_multi_slice_mode { > #define V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES (V4L2_CID_CODEC_BASE+234) > #define V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (V4L2_CID_CODEC_BASE+235) > #define V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD (V4L2_CID_CODEC_BASE+236) > +#define V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE (V4L2_CID_CODEC_BASE+237) > +enum v4l2_mpeg_video_intra_refresh_type { > + V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_RANDOM = 0, > + V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_CYCLIC = 1, > +}; > > /* CIDs for the MPEG-2 Part 2 (H.262) codec */ > #define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL (V4L2_CID_CODEC_BASE+270) Regards, Hans ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] media: v4l2-ctrls: Add intra-refresh type control 2022-02-15 8:21 ` Hans Verkuil @ 2022-02-21 6:02 ` dikshita 2022-03-08 11:21 ` Hans Verkuil 0 siblings, 1 reply; 7+ messages in thread From: dikshita @ 2022-02-21 6:02 UTC (permalink / raw) To: Hans Verkuil Cc: Dikshita Agarwal, linux-media, linux-kernel, linux-arm-msm, ezequiel, vgarodia, stanimir.varbanov, Dikshita Agarwal On 2022-02-15 13:51, Hans Verkuil wrote: > Hi Dikshita, > > Some comments below: > > On 1/24/22 11:11, Dikshita Agarwal wrote: >> From: Dikshita Agarwal <quic_dikshita@quicinc.com> >> >> Add a control to set intra-refresh type. >> >> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> >> --- >> .../userspace-api/media/v4l/ext-ctrls-codec.rst | 23 >> ++++++++++++++++++++++ >> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 9 +++++++++ >> include/uapi/linux/v4l2-controls.h | 5 +++++ >> 3 files changed, 37 insertions(+) >> >> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >> b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >> index e141f0e..54b42e1 100644 >> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >> @@ -1180,6 +1180,29 @@ enum v4l2_mpeg_video_h264_entropy_mode - >> is set to non zero value. >> Applicable to H264, H263 and MPEG4 encoder. >> >> +``V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE (enum)`` >> + >> +enum v4l2_mpeg_video_intra_refresh_type - >> + Sets the type of intra refresh. The period to refresh >> + the whole frame is specified by >> V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD. >> + Note if the client sets this control to either >> ``V4L2_MPEG_VIDEO_INTRA_REFRESH_RANDOM`` >> + or ``V4L2_MPEG_VIDEO_INTRA_REFRESH_CYCLIC`` the >> ``V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB`` >> + control shall be ignored. > > Since this control has only two possible values, that would mean that, > if this control > is present, then REFRESH_MB is always ignored. > > It seems to me that you need a third option here that specifically > selects the REFRESH_MB > method. > > Also, this needs to be documented as well in REFRESH_MB (i.e. it is > ignored if this TYPE > control is present and is set to something other than REFRESH_MB). > Hi Hans, I don't think we need to add that as the third option in this control. So, there are two ways to set intra refresh to driver, it can be either MB based or Frame-based. Currently, we have two v4l2 controls in place 1. V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB -> this is MB based and only applicable for cyclic 2. V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD -> this is frame based and has no type associated to it and it is up to the driver to decide the type i.e Random or Cyclic. with this new control V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE, we are introducing a way for the client to set the type of intra refresh, either cyclic or random. Thanks, Dikshita >> + Applicable to H264, H263 and MPEG4 encoder. Possible values are: >> + >> +.. tabularcolumns:: |p{9.6cm}|p{7.9cm}| >> + >> +.. flat-table:: >> + :header-rows: 0 >> + :stub-columns: 0 >> + >> + * - ``V4L2_MPEG_VIDEO_INTRA_REFRESH_RANDOM`` > > I think you should add _TYPE after REFRESH in these names to clearly > specify > that this is setting the refresh *type*. > >> + - The whole frame is completely refreshed randomly >> + after the specified period. >> + * - ``V4L2_MPEG_VIDEO_INTRA_REFRESH_CYCLIC`` >> + - The whole frame MBs are completely refreshed in cyclic order >> + after the specified period. >> + >> ``V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD (integer)`` >> Intra macroblock refresh period. This sets the period to refresh >> the whole frame. In other words, this defines the number of >> frames >> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c >> b/drivers/media/v4l2-core/v4l2-ctrls-defs.c >> index 54ca4e6..f13f587 100644 >> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c >> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c >> @@ -572,6 +572,11 @@ const char * const *v4l2_ctrl_get_menu(u32 id) >> "VBV/CPB Limit", >> NULL, >> }; >> + static const char * const intra_refresh_type[] = { >> + "Random", >> + "Cyclic", >> + NULL, >> + }; >> >> switch (id) { >> case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ: >> @@ -705,6 +710,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id) >> return hevc_start_code; >> case V4L2_CID_CAMERA_ORIENTATION: >> return camera_orientation; >> + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: >> + return intra_refresh_type; >> default: >> return NULL; >> } >> @@ -834,6 +841,7 @@ const char *v4l2_ctrl_get_name(u32 id) >> case V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE: return "Decoder >> Slice Interface"; >> case V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER: return "MPEG4 >> Loop Filter Enable"; >> case V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB: return "Number of >> Intra Refresh MBs"; >> + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: return "Intra Refresh >> Type"; >> case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD: return "Intra >> Refresh Period"; >> case V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE: return "Frame Level Rate >> Control Enable"; >> case V4L2_CID_MPEG_VIDEO_MB_RC_ENABLE: return "H264 MB Level Rate >> Control"; >> @@ -1360,6 +1368,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, >> enum v4l2_ctrl_type *type, >> case V4L2_CID_STATELESS_H264_DECODE_MODE: >> case V4L2_CID_STATELESS_H264_START_CODE: >> case V4L2_CID_CAMERA_ORIENTATION: >> + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: >> *type = V4L2_CTRL_TYPE_MENU; >> break; >> case V4L2_CID_LINK_FREQ: >> diff --git a/include/uapi/linux/v4l2-controls.h >> b/include/uapi/linux/v4l2-controls.h >> index c8e0f84..9650b71 100644 >> --- a/include/uapi/linux/v4l2-controls.h >> +++ b/include/uapi/linux/v4l2-controls.h >> @@ -443,6 +443,11 @@ enum v4l2_mpeg_video_multi_slice_mode { >> #define V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES (V4L2_CID_CODEC_BASE+234) >> #define >> V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (V4L2_CID_CODEC_BASE+235) >> #define >> V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD (V4L2_CID_CODEC_BASE+236) >> +#define >> V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE (V4L2_CID_CODEC_BASE+237) >> +enum v4l2_mpeg_video_intra_refresh_type { >> + V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_RANDOM = 0, >> + V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_CYCLIC = 1, >> +}; >> >> /* CIDs for the MPEG-2 Part 2 (H.262) codec */ >> #define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL (V4L2_CID_CODEC_BASE+270) > > Regards, > > Hans ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] media: v4l2-ctrls: Add intra-refresh type control 2022-02-21 6:02 ` dikshita @ 2022-03-08 11:21 ` Hans Verkuil 0 siblings, 0 replies; 7+ messages in thread From: Hans Verkuil @ 2022-03-08 11:21 UTC (permalink / raw) To: dikshita Cc: Dikshita Agarwal, linux-media, linux-kernel, linux-arm-msm, ezequiel, vgarodia, stanimir.varbanov, Dikshita Agarwal Hi Dikshita, On 2/21/22 07:02, dikshita@codeaurora.org wrote: > On 2022-02-15 13:51, Hans Verkuil wrote: >> Hi Dikshita, >> >> Some comments below: >> >> On 1/24/22 11:11, Dikshita Agarwal wrote: >>> From: Dikshita Agarwal <quic_dikshita@quicinc.com> >>> >>> Add a control to set intra-refresh type. >>> >>> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> >>> --- >>> .../userspace-api/media/v4l/ext-ctrls-codec.rst | 23 >>> ++++++++++++++++++++++ >>> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 9 +++++++++ >>> include/uapi/linux/v4l2-controls.h | 5 +++++ >>> 3 files changed, 37 insertions(+) >>> >>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >>> b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >>> index e141f0e..54b42e1 100644 >>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >>> @@ -1180,6 +1180,29 @@ enum v4l2_mpeg_video_h264_entropy_mode - >>> is set to non zero value. >>> Applicable to H264, H263 and MPEG4 encoder. >>> >>> +``V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE (enum)`` >>> + >>> +enum v4l2_mpeg_video_intra_refresh_type - >>> + Sets the type of intra refresh. The period to refresh >>> + the whole frame is specified by >>> V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD. >>> + Note if the client sets this control to either >>> ``V4L2_MPEG_VIDEO_INTRA_REFRESH_RANDOM`` >>> + or ``V4L2_MPEG_VIDEO_INTRA_REFRESH_CYCLIC`` the >>> ``V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB`` >>> + control shall be ignored. >> >> Since this control has only two possible values, that would mean that, >> if this control >> is present, then REFRESH_MB is always ignored. >> >> It seems to me that you need a third option here that specifically >> selects the REFRESH_MB >> method. >> >> Also, this needs to be documented as well in REFRESH_MB (i.e. it is >> ignored if this TYPE >> control is present and is set to something other than REFRESH_MB). >> > > Hi Hans, > > I don't think we need to add that as the third option in this control. > > So, there are two ways to set intra refresh to driver, it can be either > MB based or Frame-based. > Currently, we have two v4l2 controls in place > 1. V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB -> this is MB based and > only applicable for cyclic > 2. V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD -> this is frame based and > has no type associated to it > and it is up to the driver to decide the type i.e Random or Cyclic. > > with this new control V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE, we are > introducing > a way for the client to set the type of intra refresh, either cyclic or > random. Ah, right. But then it really is a naming issue that causes the confusion. It would all be clear if you name this control V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD_TYPE and the enums V4L2_MPEG_VIDEO_INTRA_REFRESH_PERIOD_TYPE_RANDOM and V4L2_MPEG_VIDEO_INTRA_REFRESH_PERIOD_TYPE_CYCLIC. Yes, I know, the names are quite long, but there are so many codec controls that being descriptive is more important than being concise. With these new names it is clear that this type control only relates to the refresh period and not to the refresh MB. You should also document that if this type control is not present, then it is undefined what refresh period type is used (or something along those lines). Regards, Hans > > Thanks, > Dikshita > >>> + Applicable to H264, H263 and MPEG4 encoder. Possible values are: >>> + >>> +.. tabularcolumns:: |p{9.6cm}|p{7.9cm}| >>> + >>> +.. flat-table:: >>> + :header-rows: 0 >>> + :stub-columns: 0 >>> + >>> + * - ``V4L2_MPEG_VIDEO_INTRA_REFRESH_RANDOM`` >> >> I think you should add _TYPE after REFRESH in these names to clearly >> specify >> that this is setting the refresh *type*. >> >>> + - The whole frame is completely refreshed randomly >>> + after the specified period. >>> + * - ``V4L2_MPEG_VIDEO_INTRA_REFRESH_CYCLIC`` >>> + - The whole frame MBs are completely refreshed in cyclic order >>> + after the specified period. >>> + >>> ``V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD (integer)`` >>> Intra macroblock refresh period. This sets the period to refresh >>> the whole frame. In other words, this defines the number of >>> frames >>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c >>> b/drivers/media/v4l2-core/v4l2-ctrls-defs.c >>> index 54ca4e6..f13f587 100644 >>> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c >>> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c >>> @@ -572,6 +572,11 @@ const char * const *v4l2_ctrl_get_menu(u32 id) >>> "VBV/CPB Limit", >>> NULL, >>> }; >>> + static const char * const intra_refresh_type[] = { >>> + "Random", >>> + "Cyclic", >>> + NULL, >>> + }; >>> >>> switch (id) { >>> case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ: >>> @@ -705,6 +710,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id) >>> return hevc_start_code; >>> case V4L2_CID_CAMERA_ORIENTATION: >>> return camera_orientation; >>> + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: >>> + return intra_refresh_type; >>> default: >>> return NULL; >>> } >>> @@ -834,6 +841,7 @@ const char *v4l2_ctrl_get_name(u32 id) >>> case V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE: return "Decoder >>> Slice Interface"; >>> case V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER: return "MPEG4 >>> Loop Filter Enable"; >>> case V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB: return "Number of >>> Intra Refresh MBs"; >>> + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: return "Intra Refresh >>> Type"; >>> case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD: return "Intra >>> Refresh Period"; >>> case V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE: return "Frame Level Rate >>> Control Enable"; >>> case V4L2_CID_MPEG_VIDEO_MB_RC_ENABLE: return "H264 MB Level Rate >>> Control"; >>> @@ -1360,6 +1368,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, >>> enum v4l2_ctrl_type *type, >>> case V4L2_CID_STATELESS_H264_DECODE_MODE: >>> case V4L2_CID_STATELESS_H264_START_CODE: >>> case V4L2_CID_CAMERA_ORIENTATION: >>> + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: >>> *type = V4L2_CTRL_TYPE_MENU; >>> break; >>> case V4L2_CID_LINK_FREQ: >>> diff --git a/include/uapi/linux/v4l2-controls.h >>> b/include/uapi/linux/v4l2-controls.h >>> index c8e0f84..9650b71 100644 >>> --- a/include/uapi/linux/v4l2-controls.h >>> +++ b/include/uapi/linux/v4l2-controls.h >>> @@ -443,6 +443,11 @@ enum v4l2_mpeg_video_multi_slice_mode { >>> #define V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES (V4L2_CID_CODEC_BASE+234) >>> #define >>> V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (V4L2_CID_CODEC_BASE+235) >>> #define >>> V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD (V4L2_CID_CODEC_BASE+236) >>> +#define >>> V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE (V4L2_CID_CODEC_BASE+237) >>> +enum v4l2_mpeg_video_intra_refresh_type { >>> + V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_RANDOM = 0, >>> + V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_CYCLIC = 1, >>> +}; >>> >>> /* CIDs for the MPEG-2 Part 2 (H.262) codec */ >>> #define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL (V4L2_CID_CODEC_BASE+270) >> >> Regards, >> >> Hans ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 2/2] venus: venc: Add support for intra-refresh mode 2022-01-24 10:11 [PATCH v1 0/2] Intra-refresh type control Dikshita Agarwal 2022-01-24 10:11 ` [PATCH v1 1/2] media: v4l2-ctrls: Add intra-refresh " Dikshita Agarwal @ 2022-01-24 10:11 ` Dikshita Agarwal 1 sibling, 0 replies; 7+ messages in thread From: Dikshita Agarwal @ 2022-01-24 10:11 UTC (permalink / raw) To: linux-media, linux-kernel, linux-arm-msm Cc: hverkuil-cisco, ezequiel, vgarodia, stanimir.varbanov, Dikshita Agarwal From: Dikshita Agarwal <quic_dikshita@quicinc.com> Add support for intra-refresh type v4l2 control. Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> --- drivers/media/platform/qcom/venus/core.h | 1 + drivers/media/platform/qcom/venus/venc.c | 3 ++- drivers/media/platform/qcom/venus/venc_ctrls.c | 8 ++++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h index 7c3bac0..814ec3c 100644 --- a/drivers/media/platform/qcom/venus/core.h +++ b/drivers/media/platform/qcom/venus/core.h @@ -260,6 +260,7 @@ struct venc_controls { u32 header_mode; bool aud_enable; + u32 intra_refresh_mode; u32 intra_refresh_period; struct { diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c index 84bafc3..e8b8135 100644 --- a/drivers/media/platform/qcom/venus/venc.c +++ b/drivers/media/platform/qcom/venus/venc.c @@ -893,8 +893,9 @@ static int venc_set_properties(struct venus_inst *inst) mbs++; mbs /= ctr->intra_refresh_period; - intra_refresh.mode = HFI_INTRA_REFRESH_RANDOM; intra_refresh.cir_mbs = mbs; + if (ctr->intra_refresh_mode == V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_RANDOM) + intra_refresh.mode = HFI_INTRA_REFRESH_RANDOM; } ptype = HFI_PROPERTY_PARAM_VENC_INTRA_REFRESH; diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c index 1ada42d..c54b46f 100644 --- a/drivers/media/platform/qcom/venus/venc_ctrls.c +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c @@ -316,6 +316,9 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) case V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY: ctr->mastering = *ctrl->p_new.p_hdr10_mastering; break; + case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE: + ctr->intra_refresh_mode = ctrl->val; + break; case V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD: ctr->intra_refresh_period = ctrl->val; break; @@ -582,6 +585,11 @@ int venc_ctrl_init(struct venus_inst *inst) V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY, v4l2_ctrl_ptr_create(NULL)); + v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops, + V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_TYPE, + V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_RANDOM, + 0, V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_RANDOM); + v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops, V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD, 0, ((4096 * 2304) >> 8), 1, 0); -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-03-08 11:21 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-01-24 10:11 [PATCH v1 0/2] Intra-refresh type control Dikshita Agarwal 2022-01-24 10:11 ` [PATCH v1 1/2] media: v4l2-ctrls: Add intra-refresh " Dikshita Agarwal 2022-01-25 20:10 ` Nicolas Dufresne 2022-02-15 8:21 ` Hans Verkuil 2022-02-21 6:02 ` dikshita 2022-03-08 11:21 ` Hans Verkuil 2022-01-24 10:11 ` [PATCH v1 2/2] venus: venc: Add support for intra-refresh mode Dikshita Agarwal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox