Hello,
Suggested by Hans at [1], this patchset is promoting a standard control
for frame skip mode.
The original private V4L2_CID_MPEG_MFC51_VIDEO_FRAME_SKIP_MODE control is
applicable and can be used by Venus driver too (and probably other drivers).
In order to make that possible make a new V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE
standard menu control (a copy of the private one).
Also, to keep mfc driver backward compatible kept the private one, and mark
it as depricated in docs.
regards,
Stan
[1] https://lkml.org/lkml/2020/5/19/122
Stanimir Varbanov (4):
media: v4l2-ctrl: Add frame-skip std encoder control
venus: venc: Add support for frame-skip mode v4l2 control
media: s5p-mfc: Use standard frame skip mode control
media: docs: Depricate mfc frame skip control
.../media/v4l/ext-ctrls-codec.rst | 37 +++++++++++++++++++
drivers/media/platform/qcom/venus/core.h | 1 +
drivers/media/platform/qcom/venus/venc.c | 6 ++-
.../media/platform/qcom/venus/venc_ctrls.c | 12 +++++-
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 6 +++
drivers/media/v4l2-core/v4l2-ctrls.c | 10 +++++
include/uapi/linux/v4l2-controls.h | 6 +++
7 files changed, 75 insertions(+), 3 deletions(-)
--
2.17.1
Adds encoders standard v4l2 control for frame-skip. The control
is a copy of a custom encoder control so that other v4l2 encoder
drivers can use it.
Signed-off-by: Stanimir Varbanov <[email protected]>
---
.../media/v4l/ext-ctrls-codec.rst | 32 +++++++++++++++++++
drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++
include/uapi/linux/v4l2-controls.h | 6 ++++
3 files changed, 48 insertions(+)
diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
index d0d506a444b1..a8b4c0b40747 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
@@ -592,6 +592,38 @@ enum v4l2_mpeg_video_bitrate_mode -
the average video bitrate. It is ignored if the video bitrate mode
is set to constant bitrate.
+``V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE (enum)``
+
+enum v4l2_mpeg_video_frame_skip_mode -
+ Indicates in what conditions the encoder should skip frames. If
+ encoding a frame would cause the encoded stream to be larger then a
+ chosen data limit then the frame will be skipped. Possible values
+ are:
+
+
+.. tabularcolumns:: |p{9.2cm}|p{8.3cm}|
+
+.. raw:: latex
+
+ \small
+
+.. flat-table::
+ :header-rows: 0
+ :stub-columns: 0
+
+ * - ``V4L2_MPEG_FRAME_SKIP_MODE_DISABLED``
+ - Frame skip mode is disabled.
+ * - ``V4L2_MPEG_FRAME_SKIP_MODE_LEVEL_LIMIT``
+ - Frame skip mode enabled and buffer limit is set by the chosen
+ level and is defined by the standard.
+ * - ``V4L2_MPEG_FRAME_SKIP_MODE_BUF_LIMIT``
+ - Frame skip mode enabled and buffer limit is set by the VBV
+ (MPEG1/2/4) or CPB (H264) buffer size control.
+
+.. raw:: latex
+
+ \normalsize
+
``V4L2_CID_MPEG_VIDEO_TEMPORAL_DECIMATION (integer)``
For every captured frame, skip this many subsequent frames (default
0).
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 3f3fbcd60cc6..d088acfa6dd8 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -590,6 +590,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
"External",
NULL,
};
+ static const char * const mpeg_video_frame_skip[] = {
+ "Disabled",
+ "Level Limit",
+ "VBV/CPB Limit",
+ NULL,
+ };
switch (id) {
case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
@@ -651,6 +657,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
return flash_strobe_source;
case V4L2_CID_MPEG_VIDEO_HEADER_MODE:
return header_mode;
+ case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE:
+ return mpeg_video_frame_skip;
case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE:
return multi_slice;
case V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE:
@@ -844,6 +852,7 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_MPEG_VIDEO_MB_RC_ENABLE: return "H264 MB Level Rate Control";
case V4L2_CID_MPEG_VIDEO_HEADER_MODE: return "Sequence Header Mode";
case V4L2_CID_MPEG_VIDEO_MAX_REF_PIC: return "Max Number of Reference Pics";
+ case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE: return "Frame Skip Mode";
case V4L2_CID_MPEG_VIDEO_H263_I_FRAME_QP: return "H263 I-Frame QP Value";
case V4L2_CID_MPEG_VIDEO_H263_P_FRAME_QP: return "H263 P-Frame QP Value";
case V4L2_CID_MPEG_VIDEO_H263_B_FRAME_QP: return "H263 B-Frame QP Value";
@@ -1265,6 +1274,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
case V4L2_CID_FLASH_LED_MODE:
case V4L2_CID_FLASH_STROBE_SOURCE:
case V4L2_CID_MPEG_VIDEO_HEADER_MODE:
+ case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE:
case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE:
case V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE:
case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 62271418c1be..4e1526175a4c 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -742,6 +742,12 @@ enum v4l2_cid_mpeg_video_hevc_size_of_length_field {
#define V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_BR (V4L2_CID_MPEG_BASE + 642)
#define V4L2_CID_MPEG_VIDEO_REF_NUMBER_FOR_PFRAMES (V4L2_CID_MPEG_BASE + 643)
#define V4L2_CID_MPEG_VIDEO_PREPEND_SPSPPS_TO_IDR (V4L2_CID_MPEG_BASE + 644)
+#define V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE (V4L2_CID_MPEG_BASE + 645)
+enum v4l2_mpeg_video_frame_skip_mode {
+ V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_DISABLED = 0,
+ V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_LEVEL_LIMIT = 1,
+ V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_BUF_LIMIT = 2,
+};
/* MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
#define V4L2_CID_MPEG_CX2341X_BASE (V4L2_CTRL_CLASS_MPEG | 0x1000)
--
2.17.1
Depricate mfc private frame skip mode control for new
clients and use the standard one instead.
Signed-off-by: Stanimir Varbanov <[email protected]>
---
Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
index a8b4c0b40747..c0760bfc54d4 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
@@ -2805,6 +2805,11 @@ MFC 5.1 Control IDs
``V4L2_CID_MPEG_MFC51_VIDEO_FRAME_SKIP_MODE``
(enum)
+ .. note::
+
+ This control is depricated. Use the standard one
+ ``V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE`` instead.
+
enum v4l2_mpeg_mfc51_video_frame_skip_mode -
Indicates in what conditions the encoder should skip frames. If
encoding a frame would cause the encoded stream to be larger then a
--
2.17.1
This adds support for frame-skip-mode standard v4l2 control in
encoder driver. The control is implemented based on the
combination of client selected bitrate-mode and frame-skip-mode.
Once The client selected bitrate-mode (constant or variable) and
the frame-skip-mode is not disabled we set variable framerate for
rate controller.
Signed-off-by: Stanimir Varbanov <[email protected]>
---
drivers/media/platform/qcom/venus/core.h | 1 +
drivers/media/platform/qcom/venus/venc.c | 6 ++++--
drivers/media/platform/qcom/venus/venc_ctrls.c | 12 +++++++++++-
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 7118612673c9..5e74d0441592 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -201,6 +201,7 @@ struct venc_controls {
u32 bitrate;
u32 bitrate_peak;
u32 rc_enable;
+ u32 frame_skip_mode;
u32 h264_i_period;
u32 h264_entropy_mode;
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 513bbc07f7bc..2279596d4d60 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -739,9 +739,11 @@ static int venc_set_properties(struct venus_inst *inst)
if (!ctr->rc_enable)
rate_control = HFI_RATE_CONTROL_OFF;
else if (ctr->bitrate_mode == V4L2_MPEG_VIDEO_BITRATE_MODE_VBR)
- rate_control = HFI_RATE_CONTROL_VBR_CFR;
+ rate_control = ctr->frame_skip_mode ? HFI_RATE_CONTROL_VBR_VFR :
+ HFI_RATE_CONTROL_VBR_CFR;
else
- rate_control = HFI_RATE_CONTROL_CBR_CFR;
+ rate_control = ctr->frame_skip_mode ? HFI_RATE_CONTROL_CBR_VFR :
+ HFI_RATE_CONTROL_CBR_CFR;
ptype = HFI_PROPERTY_PARAM_VENC_RATE_CONTROL;
ret = hfi_session_set_property(inst, ptype, &rate_control);
diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c
index 8362dde7949e..a418d7d6db0c 100644
--- a/drivers/media/platform/qcom/venus/venc_ctrls.c
+++ b/drivers/media/platform/qcom/venus/venc_ctrls.c
@@ -202,6 +202,9 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE:
ctr->rc_enable = ctrl->val;
break;
+ case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE:
+ ctr->frame_skip_mode = ctrl->val;
+ break;
default:
return -EINVAL;
}
@@ -217,7 +220,7 @@ int venc_ctrl_init(struct venus_inst *inst)
{
int ret;
- ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 31);
+ ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 32);
if (ret)
return ret;
@@ -357,6 +360,13 @@ int venc_ctrl_init(struct venus_inst *inst)
v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE, 0, 1, 1, 1);
+ v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
+ V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE,
+ V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_BUF_LIMIT,
+ ~((1 << V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_DISABLED) |
+ (1 << V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_BUF_LIMIT)),
+ V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_DISABLED);
+
ret = inst->ctrl_handler.error;
if (ret)
goto err;
--
2.17.1
Use the standard menu control for frame skip mode in the MFC
driver. The legacy private menu control is kept for backward
compatibility.
Signed-off-by: Stanimir Varbanov <[email protected]>
---
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index 912fe0c5ab18..3092eb6777a5 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -261,6 +261,11 @@ static struct mfc_control controls[] = {
.menu_skip_mask = 0,
.default_value = V4L2_MPEG_MFC51_VIDEO_FRAME_SKIP_MODE_DISABLED,
},
+ {
+ .id = V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE,
+ .maximum = V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_BUF_LIMIT,
+ .default_value = V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_DISABLED,
+ },
{
.id = V4L2_CID_MPEG_MFC51_VIDEO_RC_FIXED_TARGET_BIT,
.type = V4L2_CTRL_TYPE_BOOLEAN,
@@ -1849,6 +1854,7 @@ static int s5p_mfc_enc_s_ctrl(struct v4l2_ctrl *ctrl)
p->seq_hdr_mode = ctrl->val;
break;
case V4L2_CID_MPEG_MFC51_VIDEO_FRAME_SKIP_MODE:
+ case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE:
p->frame_skip_mode = ctrl->val;
break;
case V4L2_CID_MPEG_MFC51_VIDEO_RC_FIXED_TARGET_BIT:
--
2.17.1
On 7/5/20 5:11 AM, Stanimir Varbanov wrote:
> Depricate mfc private frame skip mode control for new
Deprecate
Also in $subject.
> clients and use the standard one instead.
>
> Signed-off-by: Stanimir Varbanov <[email protected]>
> ---
> Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index a8b4c0b40747..c0760bfc54d4 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -2805,6 +2805,11 @@ MFC 5.1 Control IDs
> ``V4L2_CID_MPEG_MFC51_VIDEO_FRAME_SKIP_MODE``
> (enum)
>
> + .. note::
> +
> + This control is depricated. Use the standard one
deprecated.
> + ``V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE`` instead.
> +
> enum v4l2_mpeg_mfc51_video_frame_skip_mode -
> Indicates in what conditions the encoder should skip frames. If
> encoding a frame would cause the encoded stream to be larger then a
>
--
~Randy
Le dimanche 05 juillet 2020 à 15:11 +0300, Stanimir Varbanov a écrit :
> Adds encoders standard v4l2 control for frame-skip. The control
> is a copy of a custom encoder control so that other v4l2 encoder
> drivers can use it.
>
> Signed-off-by: Stanimir Varbanov <[email protected]>
> ---
> .../media/v4l/ext-ctrls-codec.rst | 32 +++++++++++++++++++
> drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++
> include/uapi/linux/v4l2-controls.h | 6 ++++
> 3 files changed, 48 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index d0d506a444b1..a8b4c0b40747 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -592,6 +592,38 @@ enum v4l2_mpeg_video_bitrate_mode -
> the average video bitrate. It is ignored if the video bitrate mode
> is set to constant bitrate.
>
> +``V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE (enum)``
> +
> +enum v4l2_mpeg_video_frame_skip_mode -
> + Indicates in what conditions the encoder should skip frames. If
> + encoding a frame would cause the encoded stream to be larger then a
> + chosen data limit then the frame will be skipped. Possible values
> + are:
I have nothing against this API, in fact it's really nice to generalize
as this is very common. Though, I think we are missing two things. This
documentation refer to the "chosen data limit". Is there controls to
configure these *chosen* limit ? The other issue is the vagueness of
the documented mode, see lower...
> +
> +
> +.. tabularcolumns:: |p{9.2cm}|p{8.3cm}|
> +
> +.. raw:: latex
> +
> + \small
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> +
> + * - ``V4L2_MPEG_FRAME_SKIP_MODE_DISABLED``
> + - Frame skip mode is disabled.
> + * - ``V4L2_MPEG_FRAME_SKIP_MODE_LEVEL_LIMIT``
> + - Frame skip mode enabled and buffer limit is set by the chosen
> + level and is defined by the standard.
At least for H.264, a level is compose of 3 limits. One is the maximum
number of macroblocks, this is is evidently not use for frame skipping
and already constrained in V4L2 (assuming the driver does not ignore
the level control of course). The two other limits are decoded
macroblocks/s and encoded kbits/s. Both are measure over time, which
means the M2M encoder needs to be timing aware. I think the time source
should be documented. Perhaps it is mandatory to set a frame interval
for this to work ? Or we need some timestamp to allow variable frame
interval ? (I don't think the second is really an option without
extending the API again, and confusingly, since I think we have used
the timestamp for other purpose already)
> + * - ``V4L2_MPEG_FRAME_SKIP_MODE_BUF_LIMIT``
> + - Frame skip mode enabled and buffer limit is set by the VBV
> + (MPEG1/2/4) or CPB (H264) buffer size control.
The notion of VBV an CPB is unlikely well known. If my memory is right,
these are constrained in buffering: in bytes (VBV) or bits per frame
over a window of n-frames (or the gop size for some less flexible
encoder) (CPB). I think these should be somehow chosen by application
(with controls), directly or indirectly, and documented here to ensure
we get consistent implementation across drivers.
> +
> +.. raw:: latex
> +
> + \normalsize
> +
> ``V4L2_CID_MPEG_VIDEO_TEMPORAL_DECIMATION (integer)``
> For every captured frame, skip this many subsequent frames (default
> 0).
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 3f3fbcd60cc6..d088acfa6dd8 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -590,6 +590,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> "External",
> NULL,
> };
> + static const char * const mpeg_video_frame_skip[] = {
> + "Disabled",
> + "Level Limit",
> + "VBV/CPB Limit",
> + NULL,
> + };
>
> switch (id) {
> case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
> @@ -651,6 +657,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> return flash_strobe_source;
> case V4L2_CID_MPEG_VIDEO_HEADER_MODE:
> return header_mode;
> + case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE:
> + return mpeg_video_frame_skip;
> case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE:
> return multi_slice;
> case V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE:
> @@ -844,6 +852,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_MPEG_VIDEO_MB_RC_ENABLE: return "H264 MB Level Rate Control";
> case V4L2_CID_MPEG_VIDEO_HEADER_MODE: return "Sequence Header Mode";
> case V4L2_CID_MPEG_VIDEO_MAX_REF_PIC: return "Max Number of Reference Pics";
> + case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE: return "Frame Skip Mode";
> case V4L2_CID_MPEG_VIDEO_H263_I_FRAME_QP: return "H263 I-Frame QP Value";
> case V4L2_CID_MPEG_VIDEO_H263_P_FRAME_QP: return "H263 P-Frame QP Value";
> case V4L2_CID_MPEG_VIDEO_H263_B_FRAME_QP: return "H263 B-Frame QP Value";
> @@ -1265,6 +1274,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_FLASH_LED_MODE:
> case V4L2_CID_FLASH_STROBE_SOURCE:
> case V4L2_CID_MPEG_VIDEO_HEADER_MODE:
> + case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE:
> case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE:
> case V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE:
> case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 62271418c1be..4e1526175a4c 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -742,6 +742,12 @@ enum v4l2_cid_mpeg_video_hevc_size_of_length_field {
> #define V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_BR (V4L2_CID_MPEG_BASE + 642)
> #define V4L2_CID_MPEG_VIDEO_REF_NUMBER_FOR_PFRAMES (V4L2_CID_MPEG_BASE + 643)
> #define V4L2_CID_MPEG_VIDEO_PREPEND_SPSPPS_TO_IDR (V4L2_CID_MPEG_BASE + 644)
> +#define V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE (V4L2_CID_MPEG_BASE + 645)
> +enum v4l2_mpeg_video_frame_skip_mode {
> + V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_DISABLED = 0,
> + V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_LEVEL_LIMIT = 1,
> + V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_BUF_LIMIT = 2,
> +};
>
> /* MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
> #define V4L2_CID_MPEG_CX2341X_BASE (V4L2_CTRL_CLASS_MPEG | 0x1000)
Hi Nicolas,
On 7/7/20 11:53 PM, Nicolas Dufresne wrote:
> Le dimanche 05 juillet 2020 à 15:11 +0300, Stanimir Varbanov a écrit :
>> Adds encoders standard v4l2 control for frame-skip. The control
>> is a copy of a custom encoder control so that other v4l2 encoder
>> drivers can use it.
>>
>> Signed-off-by: Stanimir Varbanov <[email protected]>
>> ---
>> .../media/v4l/ext-ctrls-codec.rst | 32 +++++++++++++++++++
>> drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++
>> include/uapi/linux/v4l2-controls.h | 6 ++++
>> 3 files changed, 48 insertions(+)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> index d0d506a444b1..a8b4c0b40747 100644
>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> @@ -592,6 +592,38 @@ enum v4l2_mpeg_video_bitrate_mode -
>> the average video bitrate. It is ignored if the video bitrate mode
>> is set to constant bitrate.
>>
>> +``V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE (enum)``
>> +
>> +enum v4l2_mpeg_video_frame_skip_mode -
>> + Indicates in what conditions the encoder should skip frames. If
>> + encoding a frame would cause the encoded stream to be larger then a
>> + chosen data limit then the frame will be skipped. Possible values
>> + are:
>
> I have nothing against this API, in fact it's really nice to generalize
> as this is very common. Though, I think we are missing two things. This
> documentation refer to the "chosen data limit". Is there controls to
> configure these *chosen* limit ? The other issue is the vagueness of
> the documented mode, see lower...
>
>> +
>> +
>> +.. tabularcolumns:: |p{9.2cm}|p{8.3cm}|
>> +
>> +.. raw:: latex
>> +
>> + \small
>> +
>> +.. flat-table::
>> + :header-rows: 0
>> + :stub-columns: 0
>> +
>> + * - ``V4L2_MPEG_FRAME_SKIP_MODE_DISABLED``
>> + - Frame skip mode is disabled.
>> + * - ``V4L2_MPEG_FRAME_SKIP_MODE_LEVEL_LIMIT``
>> + - Frame skip mode enabled and buffer limit is set by the chosen
>> + level and is defined by the standard.
>
> At least for H.264, a level is compose of 3 limits. One is the maximum
> number of macroblocks, this is is evidently not use for frame skipping
> and already constrained in V4L2 (assuming the driver does not ignore
> the level control of course). The two other limits are decoded
> macroblocks/s and encoded kbits/s. Both are measure over time, which
> means the M2M encoder needs to be timing aware. I think the time source
> should be documented. Perhaps it is mandatory to set a frame interval
> for this to work ? Or we need some timestamp to allow variable frame
> interval ? (I don't think the second is really an option without
> extending the API again, and confusingly, since I think we have used
> the timestamp for other purpose already)
Do you want to say that the encoder input timestamp, bitrate control
(V4L2_CID_MPEG_VIDEO_BITRATE) and S_PARM is not enough to describe
FRAME_SKIP_MODE_LEVEL_LIMIT mode?
>
>> + * - ``V4L2_MPEG_FRAME_SKIP_MODE_BUF_LIMIT``
>> + - Frame skip mode enabled and buffer limit is set by the VBV
>> + (MPEG1/2/4) or CPB (H264) buffer size control.
>
> The notion of VBV an CPB is unlikely well known. If my memory is right,
> these are constrained in buffering: in bytes (VBV) or bits per frame
> over a window of n-frames (or the gop size for some less flexible
> encoder) (CPB). I think these should be somehow chosen by application
> (with controls), directly or indirectly, and documented here to ensure
> we get consistent implementation across drivers.
I guess you want me to add here references to the following controls:
V4L2_CID_MPEG_VIDEO_VBV_SIZE
V4L2_CID_MPEG_VIDEO_VBV_DELAY
V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE
?
>
>> +
>> +.. raw:: latex
>> +
>> + \normalsize
>> +
>> ``V4L2_CID_MPEG_VIDEO_TEMPORAL_DECIMATION (integer)``
>> For every captured frame, skip this many subsequent frames (default
>> 0).
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index 3f3fbcd60cc6..d088acfa6dd8 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -590,6 +590,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>> "External",
>> NULL,
>> };
>> + static const char * const mpeg_video_frame_skip[] = {
>> + "Disabled",
>> + "Level Limit",
>> + "VBV/CPB Limit",
>> + NULL,
>> + };
>>
>> switch (id) {
>> case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
>> @@ -651,6 +657,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>> return flash_strobe_source;
>> case V4L2_CID_MPEG_VIDEO_HEADER_MODE:
>> return header_mode;
>> + case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE:
>> + return mpeg_video_frame_skip;
>> case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE:
>> return multi_slice;
>> case V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE:
>> @@ -844,6 +852,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>> case V4L2_CID_MPEG_VIDEO_MB_RC_ENABLE: return "H264 MB Level Rate Control";
>> case V4L2_CID_MPEG_VIDEO_HEADER_MODE: return "Sequence Header Mode";
>> case V4L2_CID_MPEG_VIDEO_MAX_REF_PIC: return "Max Number of Reference Pics";
>> + case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE: return "Frame Skip Mode";
>> case V4L2_CID_MPEG_VIDEO_H263_I_FRAME_QP: return "H263 I-Frame QP Value";
>> case V4L2_CID_MPEG_VIDEO_H263_P_FRAME_QP: return "H263 P-Frame QP Value";
>> case V4L2_CID_MPEG_VIDEO_H263_B_FRAME_QP: return "H263 B-Frame QP Value";
>> @@ -1265,6 +1274,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>> case V4L2_CID_FLASH_LED_MODE:
>> case V4L2_CID_FLASH_STROBE_SOURCE:
>> case V4L2_CID_MPEG_VIDEO_HEADER_MODE:
>> + case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE:
>> case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE:
>> case V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE:
>> case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>> index 62271418c1be..4e1526175a4c 100644
>> --- a/include/uapi/linux/v4l2-controls.h
>> +++ b/include/uapi/linux/v4l2-controls.h
>> @@ -742,6 +742,12 @@ enum v4l2_cid_mpeg_video_hevc_size_of_length_field {
>> #define V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_BR (V4L2_CID_MPEG_BASE + 642)
>> #define V4L2_CID_MPEG_VIDEO_REF_NUMBER_FOR_PFRAMES (V4L2_CID_MPEG_BASE + 643)
>> #define V4L2_CID_MPEG_VIDEO_PREPEND_SPSPPS_TO_IDR (V4L2_CID_MPEG_BASE + 644)
>> +#define V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE (V4L2_CID_MPEG_BASE + 645)
>> +enum v4l2_mpeg_video_frame_skip_mode {
>> + V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_DISABLED = 0,
>> + V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_LEVEL_LIMIT = 1,
>> + V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_BUF_LIMIT = 2,
>> +};
>>
>> /* MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
>> #define V4L2_CID_MPEG_CX2341X_BASE (V4L2_CTRL_CLASS_MPEG | 0x1000)
>
--
regards,
Stan
Le mercredi 15 juillet 2020 à 18:42 +0300, Stanimir Varbanov a écrit :
> Hi Nicolas,
>
> On 7/7/20 11:53 PM, Nicolas Dufresne wrote:
> > Le dimanche 05 juillet 2020 à 15:11 +0300, Stanimir Varbanov a écrit :
> > > Adds encoders standard v4l2 control for frame-skip. The control
> > > is a copy of a custom encoder control so that other v4l2 encoder
> > > drivers can use it.
> > >
> > > Signed-off-by: Stanimir Varbanov <[email protected]>
> > > ---
> > > .../media/v4l/ext-ctrls-codec.rst | 32 +++++++++++++++++++
> > > drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++
> > > include/uapi/linux/v4l2-controls.h | 6 ++++
> > > 3 files changed, 48 insertions(+)
> > >
> > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > index d0d506a444b1..a8b4c0b40747 100644
> > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > @@ -592,6 +592,38 @@ enum v4l2_mpeg_video_bitrate_mode -
> > > the average video bitrate. It is ignored if the video bitrate mode
> > > is set to constant bitrate.
> > >
> > > +``V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE (enum)``
> > > +
> > > +enum v4l2_mpeg_video_frame_skip_mode -
> > > + Indicates in what conditions the encoder should skip frames. If
> > > + encoding a frame would cause the encoded stream to be larger then a
> > > + chosen data limit then the frame will be skipped. Possible values
> > > + are:
> >
> > I have nothing against this API, in fact it's really nice to generalize
> > as this is very common. Though, I think we are missing two things. This
> > documentation refer to the "chosen data limit". Is there controls to
> > configure these *chosen* limit ? The other issue is the vagueness of
> > the documented mode, see lower...
> >
> > > +
> > > +
> > > +.. tabularcolumns:: |p{9.2cm}|p{8.3cm}|
> > > +
> > > +.. raw:: latex
> > > +
> > > + \small
> > > +
> > > +.. flat-table::
> > > + :header-rows: 0
> > > + :stub-columns: 0
> > > +
> > > + * - ``V4L2_MPEG_FRAME_SKIP_MODE_DISABLED``
> > > + - Frame skip mode is disabled.
> > > + * - ``V4L2_MPEG_FRAME_SKIP_MODE_LEVEL_LIMIT``
> > > + - Frame skip mode enabled and buffer limit is set by the chosen
> > > + level and is defined by the standard.
> >
> > At least for H.264, a level is compose of 3 limits. One is the maximum
> > number of macroblocks, this is is evidently not use for frame skipping
> > and already constrained in V4L2 (assuming the driver does not ignore
> > the level control of course). The two other limits are decoded
> > macroblocks/s and encoded kbits/s. Both are measure over time, which
> > means the M2M encoder needs to be timing aware. I think the time source
> > should be documented. Perhaps it is mandatory to set a frame interval
> > for this to work ? Or we need some timestamp to allow variable frame
> > interval ? (I don't think the second is really an option without
> > extending the API again, and confusingly, since I think we have used
> > the timestamp for other purpose already)
>
> Do you want to say that the encoder input timestamp, bitrate control
> (V4L2_CID_MPEG_VIDEO_BITRATE) and S_PARM is not enough to describe
> FRAME_SKIP_MODE_LEVEL_LIMIT mode?
I don't think we have spec to give the input timestamp a meaning that
driver can interpret. In fact I think we gave it a meaning that the
driver must not interpret it (aka driver opaque). So remain S_PARM to
give a clue, but some stream don't have a framerate (like RTP streams,
unless written in bitstream).
>
> > > + * - ``V4L2_MPEG_FRAME_SKIP_MODE_BUF_LIMIT``
> > > + - Frame skip mode enabled and buffer limit is set by the VBV
> > > + (MPEG1/2/4) or CPB (H264) buffer size control.
> >
> > The notion of VBV an CPB is unlikely well known. If my memory is right,
> > these are constrained in buffering: in bytes (VBV) or bits per frame
> > over a window of n-frames (or the gop size for some less flexible
> > encoder) (CPB). I think these should be somehow chosen by application
> > (with controls), directly or indirectly, and documented here to ensure
> > we get consistent implementation across drivers.
>
> I guess you want me to add here references to the following controls:
>
> V4L2_CID_MPEG_VIDEO_VBV_SIZE
> V4L2_CID_MPEG_VIDEO_VBV_DELAY
> V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE
Perhaps it would be helpful yes.
>
> ?
>
> > > +
> > > +.. raw:: latex
> > > +
> > > + \normalsize
> > > +
> > > ``V4L2_CID_MPEG_VIDEO_TEMPORAL_DECIMATION (integer)``
> > > For every captured frame, skip this many subsequent frames (default
> > > 0).
> > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> > > index 3f3fbcd60cc6..d088acfa6dd8 100644
> > > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > > @@ -590,6 +590,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> > > "External",
> > > NULL,
> > > };
> > > + static const char * const mpeg_video_frame_skip[] = {
> > > + "Disabled",
> > > + "Level Limit",
> > > + "VBV/CPB Limit",
> > > + NULL,
> > > + };
> > >
> > > switch (id) {
> > > case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
> > > @@ -651,6 +657,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> > > return flash_strobe_source;
> > > case V4L2_CID_MPEG_VIDEO_HEADER_MODE:
> > > return header_mode;
> > > + case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE:
> > > + return mpeg_video_frame_skip;
> > > case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE:
> > > return multi_slice;
> > > case V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE:
> > > @@ -844,6 +852,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> > > case V4L2_CID_MPEG_VIDEO_MB_RC_ENABLE: return "H264 MB Level Rate Control";
> > > case V4L2_CID_MPEG_VIDEO_HEADER_MODE: return "Sequence Header Mode";
> > > case V4L2_CID_MPEG_VIDEO_MAX_REF_PIC: return "Max Number of Reference Pics";
> > > + case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE: return "Frame Skip Mode";
> > > case V4L2_CID_MPEG_VIDEO_H263_I_FRAME_QP: return "H263 I-Frame QP Value";
> > > case V4L2_CID_MPEG_VIDEO_H263_P_FRAME_QP: return "H263 P-Frame QP Value";
> > > case V4L2_CID_MPEG_VIDEO_H263_B_FRAME_QP: return "H263 B-Frame QP Value";
> > > @@ -1265,6 +1274,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> > > case V4L2_CID_FLASH_LED_MODE:
> > > case V4L2_CID_FLASH_STROBE_SOURCE:
> > > case V4L2_CID_MPEG_VIDEO_HEADER_MODE:
> > > + case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE:
> > > case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE:
> > > case V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE:
> > > case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
> > > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > > index 62271418c1be..4e1526175a4c 100644
> > > --- a/include/uapi/linux/v4l2-controls.h
> > > +++ b/include/uapi/linux/v4l2-controls.h
> > > @@ -742,6 +742,12 @@ enum v4l2_cid_mpeg_video_hevc_size_of_length_field {
> > > #define V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_BR (V4L2_CID_MPEG_BASE + 642)
> > > #define V4L2_CID_MPEG_VIDEO_REF_NUMBER_FOR_PFRAMES (V4L2_CID_MPEG_BASE + 643)
> > > #define V4L2_CID_MPEG_VIDEO_PREPEND_SPSPPS_TO_IDR (V4L2_CID_MPEG_BASE + 644)
> > > +#define V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE (V4L2_CID_MPEG_BASE + 645)
> > > +enum v4l2_mpeg_video_frame_skip_mode {
> > > + V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_DISABLED = 0,
> > > + V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_LEVEL_LIMIT = 1,
> > > + V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_BUF_LIMIT = 2,
> > > +};
> > >
> > > /* MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
> > > #define V4L2_CID_MPEG_CX2341X_BASE (V4L2_CTRL_CLASS_MPEG | 0x1000)
On 7/15/20 9:12 PM, Nicolas Dufresne wrote:
> Le mercredi 15 juillet 2020 à 18:42 +0300, Stanimir Varbanov a écrit :
>> Hi Nicolas,
>>
>> On 7/7/20 11:53 PM, Nicolas Dufresne wrote:
>>> Le dimanche 05 juillet 2020 à 15:11 +0300, Stanimir Varbanov a écrit :
>>>> Adds encoders standard v4l2 control for frame-skip. The control
>>>> is a copy of a custom encoder control so that other v4l2 encoder
>>>> drivers can use it.
>>>>
>>>> Signed-off-by: Stanimir Varbanov <[email protected]>
>>>> ---
>>>> .../media/v4l/ext-ctrls-codec.rst | 32 +++++++++++++++++++
>>>> drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++
>>>> include/uapi/linux/v4l2-controls.h | 6 ++++
>>>> 3 files changed, 48 insertions(+)
>>>>
>>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>> index d0d506a444b1..a8b4c0b40747 100644
>>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>> @@ -592,6 +592,38 @@ enum v4l2_mpeg_video_bitrate_mode -
>>>> the average video bitrate. It is ignored if the video bitrate mode
>>>> is set to constant bitrate.
>>>>
>>>> +``V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE (enum)``
>>>> +
>>>> +enum v4l2_mpeg_video_frame_skip_mode -
>>>> + Indicates in what conditions the encoder should skip frames. If
>>>> + encoding a frame would cause the encoded stream to be larger then a
>>>> + chosen data limit then the frame will be skipped. Possible values
>>>> + are:
>>>
>>> I have nothing against this API, in fact it's really nice to generalize
>>> as this is very common. Though, I think we are missing two things. This
>>> documentation refer to the "chosen data limit". Is there controls to
>>> configure these *chosen* limit ? The other issue is the vagueness of
>>> the documented mode, see lower...
>>>
>>>> +
>>>> +
>>>> +.. tabularcolumns:: |p{9.2cm}|p{8.3cm}|
>>>> +
>>>> +.. raw:: latex
>>>> +
>>>> + \small
>>>> +
>>>> +.. flat-table::
>>>> + :header-rows: 0
>>>> + :stub-columns: 0
>>>> +
>>>> + * - ``V4L2_MPEG_FRAME_SKIP_MODE_DISABLED``
>>>> + - Frame skip mode is disabled.
>>>> + * - ``V4L2_MPEG_FRAME_SKIP_MODE_LEVEL_LIMIT``
>>>> + - Frame skip mode enabled and buffer limit is set by the chosen
>>>> + level and is defined by the standard.
>>>
>>> At least for H.264, a level is compose of 3 limits. One is the maximum
>>> number of macroblocks, this is is evidently not use for frame skipping
>>> and already constrained in V4L2 (assuming the driver does not ignore
>>> the level control of course). The two other limits are decoded
>>> macroblocks/s and encoded kbits/s. Both are measure over time, which
>>> means the M2M encoder needs to be timing aware. I think the time source
>>> should be documented. Perhaps it is mandatory to set a frame interval
>>> for this to work ? Or we need some timestamp to allow variable frame
>>> interval ? (I don't think the second is really an option without
>>> extending the API again, and confusingly, since I think we have used
>>> the timestamp for other purpose already)
>>
>> Do you want to say that the encoder input timestamp, bitrate control
>> (V4L2_CID_MPEG_VIDEO_BITRATE) and S_PARM is not enough to describe
>> FRAME_SKIP_MODE_LEVEL_LIMIT mode?
>
> I don't think we have spec to give the input timestamp a meaning that
> driver can interpret. In fact I think we gave it a meaning that the
> driver must not interpret it (aka driver opaque). So remain S_PARM to
At least for Venus the timestamps are passed to the firmware and used by
encoder rate-controller.
> give a clue, but some stream don't have a framerate (like RTP streams,
> unless written in bitstream).
I think v4l2 clients should be able to guess what would be the frame
rate in such cases, no?
--
regards,
Stan
On 05/07/2020 14:11, Stanimir Varbanov wrote:
> Adds encoders standard v4l2 control for frame-skip. The control
> is a copy of a custom encoder control so that other v4l2 encoder
> drivers can use it.
>
> Signed-off-by: Stanimir Varbanov <[email protected]>
Reviewed-by: Hans Verkuil <[email protected]>
But see note at the end.
> ---
> .../media/v4l/ext-ctrls-codec.rst | 32 +++++++++++++++++++
> drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++
> include/uapi/linux/v4l2-controls.h | 6 ++++
> 3 files changed, 48 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index d0d506a444b1..a8b4c0b40747 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -592,6 +592,38 @@ enum v4l2_mpeg_video_bitrate_mode -
> the average video bitrate. It is ignored if the video bitrate mode
> is set to constant bitrate.
>
> +``V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE (enum)``
> +
> +enum v4l2_mpeg_video_frame_skip_mode -
> + Indicates in what conditions the encoder should skip frames. If
> + encoding a frame would cause the encoded stream to be larger then a
> + chosen data limit then the frame will be skipped. Possible values
> + are:
> +
> +
> +.. tabularcolumns:: |p{9.2cm}|p{8.3cm}|
> +
> +.. raw:: latex
> +
> + \small
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> +
> + * - ``V4L2_MPEG_FRAME_SKIP_MODE_DISABLED``
> + - Frame skip mode is disabled.
> + * - ``V4L2_MPEG_FRAME_SKIP_MODE_LEVEL_LIMIT``
> + - Frame skip mode enabled and buffer limit is set by the chosen
> + level and is defined by the standard.
> + * - ``V4L2_MPEG_FRAME_SKIP_MODE_BUF_LIMIT``
> + - Frame skip mode enabled and buffer limit is set by the VBV
> + (MPEG1/2/4) or CPB (H264) buffer size control.
> +
> +.. raw:: latex
> +
> + \normalsize
> +
> ``V4L2_CID_MPEG_VIDEO_TEMPORAL_DECIMATION (integer)``
> For every captured frame, skip this many subsequent frames (default
> 0).
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 3f3fbcd60cc6..d088acfa6dd8 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -590,6 +590,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> "External",
> NULL,
> };
> + static const char * const mpeg_video_frame_skip[] = {
> + "Disabled",
> + "Level Limit",
> + "VBV/CPB Limit",
> + NULL,
> + };
>
> switch (id) {
> case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
> @@ -651,6 +657,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> return flash_strobe_source;
> case V4L2_CID_MPEG_VIDEO_HEADER_MODE:
> return header_mode;
> + case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE:
> + return mpeg_video_frame_skip;
> case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE:
> return multi_slice;
> case V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE:
> @@ -844,6 +852,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_MPEG_VIDEO_MB_RC_ENABLE: return "H264 MB Level Rate Control";
> case V4L2_CID_MPEG_VIDEO_HEADER_MODE: return "Sequence Header Mode";
> case V4L2_CID_MPEG_VIDEO_MAX_REF_PIC: return "Max Number of Reference Pics";
> + case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE: return "Frame Skip Mode";
> case V4L2_CID_MPEG_VIDEO_H263_I_FRAME_QP: return "H263 I-Frame QP Value";
> case V4L2_CID_MPEG_VIDEO_H263_P_FRAME_QP: return "H263 P-Frame QP Value";
> case V4L2_CID_MPEG_VIDEO_H263_B_FRAME_QP: return "H263 B-Frame QP Value";
> @@ -1265,6 +1274,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_FLASH_LED_MODE:
> case V4L2_CID_FLASH_STROBE_SOURCE:
> case V4L2_CID_MPEG_VIDEO_HEADER_MODE:
> + case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE:
> case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE:
> case V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE:
> case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 62271418c1be..4e1526175a4c 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -742,6 +742,12 @@ enum v4l2_cid_mpeg_video_hevc_size_of_length_field {
> #define V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_BR (V4L2_CID_MPEG_BASE + 642)
> #define V4L2_CID_MPEG_VIDEO_REF_NUMBER_FOR_PFRAMES (V4L2_CID_MPEG_BASE + 643)
> #define V4L2_CID_MPEG_VIDEO_PREPEND_SPSPPS_TO_IDR (V4L2_CID_MPEG_BASE + 644)
> +#define V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE (V4L2_CID_MPEG_BASE + 645)
I think this now clashes with "media: v4l2-ctrls: Add encoder constant quality control".
I recommend making a new series that combines both series. That avoid this problem.
Regards,
Hans
> +enum v4l2_mpeg_video_frame_skip_mode {
> + V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_DISABLED = 0,
> + V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_LEVEL_LIMIT = 1,
> + V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_BUF_LIMIT = 2,
> +};
>
> /* MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
> #define V4L2_CID_MPEG_CX2341X_BASE (V4L2_CTRL_CLASS_MPEG | 0x1000)
>
On 05/07/2020 14:11, Stanimir Varbanov wrote:
> Depricate mfc private frame skip mode control for new
Depricate -> Deprecate (same in the patch below).
> clients and use the standard one instead.
>
> Signed-off-by: Stanimir Varbanov <[email protected]>
> ---
> Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index a8b4c0b40747..c0760bfc54d4 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -2805,6 +2805,11 @@ MFC 5.1 Control IDs
> ``V4L2_CID_MPEG_MFC51_VIDEO_FRAME_SKIP_MODE``
> (enum)
>
> + .. note::
> +
> + This control is depricated. Use the standard one
s/one//
> + ``V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE`` instead.
s/instead/control instead/
> +
> enum v4l2_mpeg_mfc51_video_frame_skip_mode -
> Indicates in what conditions the encoder should skip frames. If
> encoding a frame would cause the encoded stream to be larger then a
>