* [PATCH 0/5] Stateful Encoding: final bits
@ 2019-11-19 11:34 Hans Verkuil
2019-11-19 11:34 ` [PATCH 1/5] v4l2-ctrls: add support for v4l2_fract types Hans Verkuil
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-11-19 11:34 UTC (permalink / raw)
To: linux-media; +Cc: Michael Tretter
This series adds support for fractions in the control framework,
and a way to obtain the min and max values of compound controls
such as v4l2_fract.
Next it adds the V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE control to
set the framerate for the encoder.
The next patch adds support for the V4L2_BUF_FLAG_TOO_SMALL flag
to signal that the capture buffer was too small.
The final patch adds the encoder spec (unchanged from v3).
Michael, can you add support for V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE
to your encoder driver? Let me know if something isn't working.
I need to add a test control for this to vivid as well and add support
for this to v4l2-ctl, that's on my TODO list.
Open questions:
1) Existing encoder drivers use S_PARM to signal the frameperiod,
but as discussed in Lyon this should be the rate at which frames are
submitted for encoding, which can be different from
V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE. But given the current use-cases
I was wondering if calling S_PARM should set the ENC_FRAME_RATE
control as well, and you would need to explicitly set the control
after S_PARM if the two are not the same. This would mean that
existing applications that don't know about the control can keep working.
2) I do believe that we need a RELEASE/DEL/DESTROY_BUFS ioctl in
order to do proper handling of the V4L2_BUF_FLAG_TOO_SMALL case.
I am inclined to postpone adding this flag until we have that ioctl.
We can still merge the stateful encoder spec if we mention that that
is work in progress.
Regards,
Hans
Hans Verkuil (4):
v4l2-ctrls: add support for v4l2_fract types
v4l2-ctrls: add support for V4L2_CTRL_WHICH_MIN/MAX_VAL
v4l2-controls.h: add V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE
videodev2.h: add V4L2_BUF_FLAG_TOO_SMALL flag
Tomasz Figa (1):
media: docs-rst: Document memory-to-memory video encoder interface
Documentation/media/uapi/v4l/buffer.rst | 9 +
Documentation/media/uapi/v4l/dev-encoder.rst | 608 ++++++++++++++++++
Documentation/media/uapi/v4l/dev-mem2mem.rst | 1 +
.../media/uapi/v4l/ext-ctrls-codec.rst | 8 +
Documentation/media/uapi/v4l/pixfmt-v4l2.rst | 5 +
Documentation/media/uapi/v4l/v4l2.rst | 2 +
.../media/uapi/v4l/vidioc-encoder-cmd.rst | 51 +-
.../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 15 +-
.../media/uapi/v4l/vidioc-queryctrl.rst | 6 +
.../media/videodev2.h.rst.exceptions | 3 +
.../media/common/videobuf2/videobuf2-core.c | 12 +-
.../media/common/videobuf2/videobuf2-v4l2.c | 4 +
drivers/media/i2c/imx214.c | 4 +-
drivers/media/v4l2-core/v4l2-ctrls.c | 222 ++++++-
include/media/v4l2-ctrls.h | 72 ++-
include/media/videobuf2-core.h | 4 +
include/uapi/linux/v4l2-controls.h | 1 +
include/uapi/linux/videodev2.h | 6 +
18 files changed, 980 insertions(+), 53 deletions(-)
create mode 100644 Documentation/media/uapi/v4l/dev-encoder.rst
--
2.23.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/5] v4l2-ctrls: add support for v4l2_fract types
2019-11-19 11:34 [PATCH 0/5] Stateful Encoding: final bits Hans Verkuil
@ 2019-11-19 11:34 ` Hans Verkuil
2019-11-19 11:34 ` [PATCH 2/5] v4l2-ctrls: add support for V4L2_CTRL_WHICH_MIN/MAX_VAL Hans Verkuil
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-11-19 11:34 UTC (permalink / raw)
To: linux-media; +Cc: Michael Tretter, Hans Verkuil
The stateful encoder drivers need a fraction type in order to set
the desired frame rate with the required precision.
Add support for this.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
.../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 5 +++
.../media/uapi/v4l/vidioc-queryctrl.rst | 6 +++
.../media/videodev2.h.rst.exceptions | 1 +
drivers/media/v4l2-core/v4l2-ctrls.c | 22 ++++++++++
include/media/v4l2-ctrls.h | 44 ++++++++++++++++++-
include/uapi/linux/videodev2.h | 2 +
6 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
index 271cac18afbb..fbb42e73b821 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
@@ -203,6 +203,11 @@ still cause this situation.
- ``p_area``
- A pointer to a struct :c:type:`v4l2_area`. Valid if this control is
of type ``V4L2_CTRL_TYPE_AREA``.
+ * -
+ - :c:type:`v4l2_fract` *
+ - ``p_fract``
+ - A pointer to a struct :c:type:`v4l2_fract`. Valid if this control is
+ of type ``V4L2_CTRL_TYPE_FRACT``.
* -
- void *
- ``ptr``
diff --git a/Documentation/media/uapi/v4l/vidioc-queryctrl.rst b/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
index 6690928e657b..d4e3d2a71f56 100644
--- a/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
+++ b/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
@@ -449,6 +449,12 @@ See also the examples in :ref:`control`.
- n/a
- A struct :c:type:`v4l2_area`, containing the width and the height
of a rectangular area. Units depend on the use case.
+ * - ``V4L2_CTRL_TYPE_FRACT``
+ - any
+ - n/a
+ - any
+ - A struct :c:type:`v4l2_fract`, containing the numerator and the
+ denominator of the fraction.
* - ``V4L2_CTRL_TYPE_H264_SPS``
- n/a
- n/a
diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
index cb6ccf91776e..49df65d162a0 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -145,6 +145,7 @@ replace symbol V4L2_CTRL_TYPE_HEVC_SPS :c:type:`v4l2_ctrl_type`
replace symbol V4L2_CTRL_TYPE_HEVC_PPS :c:type:`v4l2_ctrl_type`
replace symbol V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS :c:type:`v4l2_ctrl_type`
replace symbol V4L2_CTRL_TYPE_AREA :c:type:`v4l2_ctrl_type`
+replace symbol V4L2_CTRL_TYPE_FRACT :c:type:`v4l2_ctrl_type`
# V4L2 capability defines
replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 2928c5e0a73d..28a5d153d18c 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1716,6 +1716,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
struct v4l2_ctrl_hevc_pps *p_hevc_pps;
struct v4l2_ctrl_hevc_slice_params *p_hevc_slice_params;
struct v4l2_area *area;
+ struct v4l2_fract *fract;
void *p = ptr.p + idx * ctrl->elem_size;
unsigned int i;
@@ -1863,6 +1864,12 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
return -EINVAL;
break;
+ case V4L2_CTRL_TYPE_FRACT:
+ fract = p;
+ if (!fract->denominator)
+ return -EINVAL;
+ break;
+
default:
return -EINVAL;
}
@@ -2549,6 +2556,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
case V4L2_CTRL_TYPE_AREA:
elem_size = sizeof(struct v4l2_area);
break;
+ case V4L2_CTRL_TYPE_FRACT:
+ elem_size = sizeof(struct v4l2_fract);
+ break;
default:
if (type < V4L2_CTRL_COMPOUND_TYPES)
elem_size = sizeof(s32);
@@ -4255,6 +4265,18 @@ int __v4l2_ctrl_s_ctrl_area(struct v4l2_ctrl *ctrl,
}
EXPORT_SYMBOL(__v4l2_ctrl_s_ctrl_area);
+int __v4l2_ctrl_s_ctrl_fract(struct v4l2_ctrl *ctrl,
+ const struct v4l2_fract *fract)
+{
+ lockdep_assert_held(ctrl->handler->lock);
+
+ /* It's a driver bug if this happens. */
+ WARN_ON(ctrl->type != V4L2_CTRL_TYPE_FRACT);
+ *ctrl->p_new.p_fract = *fract;
+ return set_ctrl(NULL, ctrl, 0);
+}
+EXPORT_SYMBOL(__v4l2_ctrl_s_ctrl_fract);
+
void v4l2_ctrl_request_complete(struct media_request *req,
struct v4l2_ctrl_handler *main_hdl)
{
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 7db9e719a583..aefd62831bc8 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -54,7 +54,8 @@ struct poll_table_struct;
* @p_hevc_sps: Pointer to an HEVC sequence parameter set structure.
* @p_hevc_pps: Pointer to an HEVC picture parameter set structure.
* @p_hevc_slice_params: Pointer to an HEVC slice parameters structure.
- * @p_area: Pointer to an area.
+ * @p_area: Pointer to a struct v4l2_area.
+ * @p_fract: Pointer to a struct v4l2_fract.
* @p: Pointer to a compound value.
* @p_const: Pointer to a constant compound value.
*/
@@ -78,6 +79,7 @@ union v4l2_ctrl_ptr {
struct v4l2_ctrl_hevc_pps *p_hevc_pps;
struct v4l2_ctrl_hevc_slice_params *p_hevc_slice_params;
struct v4l2_area *p_area;
+ struct v4l2_fract *p_fract;
void *p;
const void *p_const;
};
@@ -1152,6 +1154,46 @@ static inline int v4l2_ctrl_s_ctrl_area(struct v4l2_ctrl *ctrl,
return rval;
}
+/**
+ * __v4l2_ctrl_s_ctrl_fract() - Unlocked variant of v4l2_ctrl_s_ctrl_fract().
+ *
+ * @ctrl: The control.
+ * @fract: The new fraction.
+ *
+ * This sets the control's new fraction safely by going through the control
+ * framework. This function assumes the control's handler is already locked,
+ * allowing it to be used from within the &v4l2_ctrl_ops functions.
+ *
+ * This function is for fraction type controls only.
+ */
+int __v4l2_ctrl_s_ctrl_fract(struct v4l2_ctrl *ctrl,
+ const struct v4l2_fract *fract);
+
+/**
+ * v4l2_ctrl_s_ctrl_fract() - Helper function to set a control's fraction value
+ * from within a driver.
+ *
+ * @ctrl: The control.
+ * @fract: The new fraction.
+ *
+ * This sets the control's new fraction safely by going through the control
+ * framework. This function will lock the control's handler, so it cannot be
+ * used from within the &v4l2_ctrl_ops functions.
+ *
+ * This function is for area type controls only.
+ */
+static inline int v4l2_ctrl_s_ctrl_fract(struct v4l2_ctrl *ctrl,
+ const struct v4l2_fract *fract)
+{
+ int rval;
+
+ v4l2_ctrl_lock(ctrl);
+ rval = __v4l2_ctrl_s_ctrl_fract(ctrl, fract);
+ v4l2_ctrl_unlock(ctrl);
+
+ return rval;
+}
+
/* Internal helper functions that deal with control events. */
extern const struct v4l2_subscribed_event_ops v4l2_ctrl_sub_ev_ops;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 04481c717fee..2b584c69e619 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1685,6 +1685,7 @@ struct v4l2_ext_control {
__u16 __user *p_u16;
__u32 __user *p_u32;
struct v4l2_area __user *p_area;
+ struct v4l2_fract __user *p_fract;
void __user *ptr;
};
} __attribute__ ((packed));
@@ -1731,6 +1732,7 @@ enum v4l2_ctrl_type {
V4L2_CTRL_TYPE_U16 = 0x0101,
V4L2_CTRL_TYPE_U32 = 0x0102,
V4L2_CTRL_TYPE_AREA = 0x0106,
+ V4L2_CTRL_TYPE_FRACT = 0x0107,
};
/* Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/5] v4l2-ctrls: add support for V4L2_CTRL_WHICH_MIN/MAX_VAL
2019-11-19 11:34 [PATCH 0/5] Stateful Encoding: final bits Hans Verkuil
2019-11-19 11:34 ` [PATCH 1/5] v4l2-ctrls: add support for v4l2_fract types Hans Verkuil
@ 2019-11-19 11:34 ` Hans Verkuil
2019-11-19 11:34 ` [PATCH 3/5] v4l2-controls.h: add V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE Hans Verkuil
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-11-19 11:34 UTC (permalink / raw)
To: linux-media; +Cc: Michael Tretter, Hans Verkuil
Add the capability of retrieving the min and max values of a
compound control.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
.../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 10 +-
.../media/videodev2.h.rst.exceptions | 2 +
drivers/media/i2c/imx214.c | 4 +-
drivers/media/v4l2-core/v4l2-ctrls.c | 196 ++++++++++++++++--
include/media/v4l2-ctrls.h | 28 ++-
include/uapi/linux/videodev2.h | 2 +
6 files changed, 213 insertions(+), 29 deletions(-)
diff --git a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
index fbb42e73b821..cdee57c8d3dc 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
@@ -246,7 +246,9 @@ still cause this situation.
- Which value of the control to get/set/try.
``V4L2_CTRL_WHICH_CUR_VAL`` will return the current value of the
control, ``V4L2_CTRL_WHICH_DEF_VAL`` will return the default
- value of the control and ``V4L2_CTRL_WHICH_REQUEST_VAL`` indicates that
+ value of the control, ``V4L2_CTRL_WHICH_MIN_VAL`` will return the minimum
+ value of the control, ``V4L2_CTRL_WHICH_MAX_VAL`` will return the maximum
+ value of the control and ``V4L2_CTRL_WHICH_REQUEST_VAL`` indicates that
these controls have to be retrieved from a request or tried/set for
a request. In the latter case the ``request_fd`` field contains the
file descriptor of the request that should be used. If the device
@@ -254,8 +256,10 @@ still cause this situation.
.. note::
- When using ``V4L2_CTRL_WHICH_DEF_VAL`` be aware that you can only
- get the default value of the control, you cannot set or try it.
+ When using ``V4L2_CTRL_WHICH_DEF_VAL``, ``V4L2_CTRL_WHICH_MIN_VAL``
+ or ``V4L2_CTRL_WHICH_MAX_VAL`` be aware that you can only
+ get the default/minimum/maximum value of the control, you cannot set
+ or try it.
For backwards compatibility you can also use a control class here
(see :ref:`ctrl-class`). In that case all controls have to
diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
index 49df65d162a0..e6a8ae41773d 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -536,6 +536,8 @@ ignore define V4L2_CTRL_DRIVER_PRIV
ignore define V4L2_CTRL_MAX_DIMS
ignore define V4L2_CTRL_WHICH_CUR_VAL
ignore define V4L2_CTRL_WHICH_DEF_VAL
+ignore define V4L2_CTRL_WHICH_MIN_VAL
+ignore define V4L2_CTRL_WHICH_MAX_VAL
ignore define V4L2_CTRL_WHICH_REQUEST_VAL
ignore define V4L2_OUT_CAP_CUSTOM_TIMINGS
ignore define V4L2_CID_MAX_CTRLS
diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
index adcaaa8c86d1..72938197d745 100644
--- a/drivers/media/i2c/imx214.c
+++ b/drivers/media/i2c/imx214.c
@@ -1037,7 +1037,9 @@ static int imx214_probe(struct i2c_client *client)
imx214->unit_size = v4l2_ctrl_new_std_compound(&imx214->ctrls,
NULL,
V4L2_CID_UNIT_CELL_SIZE,
- v4l2_ctrl_ptr_create((void *)&unit_size));
+ v4l2_ctrl_ptr_create((void *)&unit_size),
+ v4l2_ctrl_ptr_create(NULL),
+ v4l2_ctrl_ptr_create(NULL));
ret = imx214->ctrls.error;
if (ret) {
dev_err(&client->dev, "%s control init failed (%d)\n",
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 28a5d153d18c..af74609dce0a 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1590,6 +1590,28 @@ static void std_init_compound(const struct v4l2_ctrl *ctrl, u32 idx,
}
}
+static void std_min_compound(const struct v4l2_ctrl *ctrl, u32 idx,
+ union v4l2_ctrl_ptr ptr)
+{
+ void *p = ptr.p + idx * ctrl->elem_size;
+
+ if (ctrl->p_min.p_const)
+ memcpy(p, ctrl->p_min.p_const, ctrl->elem_size);
+ else
+ memset(p, 0, ctrl->elem_size);
+}
+
+static void std_max_compound(const struct v4l2_ctrl *ctrl, u32 idx,
+ union v4l2_ctrl_ptr ptr)
+{
+ void *p = ptr.p + idx * ctrl->elem_size;
+
+ if (ctrl->p_max.p_const)
+ memcpy(p, ctrl->p_max.p_const, ctrl->elem_size);
+ else
+ memset(p, 0, ctrl->elem_size);
+}
+
static void std_init(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr)
{
@@ -1628,6 +1650,82 @@ static void std_init(const struct v4l2_ctrl *ctrl, u32 idx,
}
}
+static void std_minimum(const struct v4l2_ctrl *ctrl, u32 idx,
+ union v4l2_ctrl_ptr ptr)
+{
+ switch (ctrl->type) {
+ case V4L2_CTRL_TYPE_STRING:
+ idx *= ctrl->elem_size;
+ memset(ptr.p_char + idx, ' ', ctrl->minimum);
+ ptr.p_char[idx + ctrl->minimum] = '\0';
+ break;
+ case V4L2_CTRL_TYPE_INTEGER64:
+ ptr.p_s64[idx] = ctrl->minimum;
+ break;
+ case V4L2_CTRL_TYPE_INTEGER:
+ case V4L2_CTRL_TYPE_INTEGER_MENU:
+ case V4L2_CTRL_TYPE_MENU:
+ case V4L2_CTRL_TYPE_BITMASK:
+ case V4L2_CTRL_TYPE_BOOLEAN:
+ ptr.p_s32[idx] = ctrl->minimum;
+ break;
+ case V4L2_CTRL_TYPE_BUTTON:
+ case V4L2_CTRL_TYPE_CTRL_CLASS:
+ ptr.p_s32[idx] = 0;
+ break;
+ case V4L2_CTRL_TYPE_U8:
+ ptr.p_u8[idx] = ctrl->minimum;
+ break;
+ case V4L2_CTRL_TYPE_U16:
+ ptr.p_u16[idx] = ctrl->minimum;
+ break;
+ case V4L2_CTRL_TYPE_U32:
+ ptr.p_u32[idx] = ctrl->minimum;
+ break;
+ default:
+ std_min_compound(ctrl, idx, ptr);
+ break;
+ }
+}
+
+static void std_maximum(const struct v4l2_ctrl *ctrl, u32 idx,
+ union v4l2_ctrl_ptr ptr)
+{
+ switch (ctrl->type) {
+ case V4L2_CTRL_TYPE_STRING:
+ idx *= ctrl->elem_size;
+ memset(ptr.p_char + idx, ' ', ctrl->maximum);
+ ptr.p_char[idx + ctrl->maximum] = '\0';
+ break;
+ case V4L2_CTRL_TYPE_INTEGER64:
+ ptr.p_s64[idx] = ctrl->maximum;
+ break;
+ case V4L2_CTRL_TYPE_INTEGER:
+ case V4L2_CTRL_TYPE_INTEGER_MENU:
+ case V4L2_CTRL_TYPE_MENU:
+ case V4L2_CTRL_TYPE_BITMASK:
+ case V4L2_CTRL_TYPE_BOOLEAN:
+ ptr.p_s32[idx] = ctrl->maximum;
+ break;
+ case V4L2_CTRL_TYPE_BUTTON:
+ case V4L2_CTRL_TYPE_CTRL_CLASS:
+ ptr.p_s32[idx] = 0;
+ break;
+ case V4L2_CTRL_TYPE_U8:
+ ptr.p_u8[idx] = ctrl->maximum;
+ break;
+ case V4L2_CTRL_TYPE_U16:
+ ptr.p_u16[idx] = ctrl->maximum;
+ break;
+ case V4L2_CTRL_TYPE_U32:
+ ptr.p_u32[idx] = ctrl->maximum;
+ break;
+ default:
+ std_max_compound(ctrl, idx, ptr);
+ break;
+ }
+}
+
static void std_log(const struct v4l2_ctrl *ctrl)
{
union v4l2_ctrl_ptr ptr = ctrl->p_cur;
@@ -1950,6 +2048,8 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
static const struct v4l2_ctrl_type_ops std_type_ops = {
.equal = std_equal,
.init = std_init,
+ .minimum = std_minimum,
+ .maximum = std_maximum,
.log = std_log,
.validate = std_validate,
};
@@ -2016,6 +2116,28 @@ static int def_to_user(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl)
return ptr_to_user(c, ctrl, ctrl->p_new);
}
+/* Helper function: copy the minimum control value back to the caller */
+static int min_to_user(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl)
+{
+ int idx;
+
+ for (idx = 0; idx < ctrl->elems; idx++)
+ ctrl->type_ops->minimum(ctrl, idx, ctrl->p_new);
+
+ return ptr_to_user(c, ctrl, ctrl->p_new);
+}
+
+/* Helper function: copy the maximum control value back to the caller */
+static int max_to_user(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl)
+{
+ int idx;
+
+ for (idx = 0; idx < ctrl->elems; idx++)
+ ctrl->type_ops->maximum(ctrl, idx, ctrl->p_new);
+
+ return ptr_to_user(c, ctrl, ctrl->p_new);
+}
+
/* Helper function: copy the caller-provider value to the given control value */
static int user_to_ptr(struct v4l2_ext_control *c,
struct v4l2_ctrl *ctrl,
@@ -2476,7 +2598,10 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
s64 min, s64 max, u64 step, s64 def,
const u32 dims[V4L2_CTRL_MAX_DIMS], u32 elem_size,
u32 flags, const char * const *qmenu,
- const s64 *qmenu_int, const union v4l2_ctrl_ptr p_def,
+ const s64 *qmenu_int,
+ const union v4l2_ctrl_ptr p_def,
+ const union v4l2_ctrl_ptr p_min,
+ const union v4l2_ctrl_ptr p_max,
void *priv)
{
struct v4l2_ctrl *ctrl;
@@ -2599,7 +2724,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
sz_extra += 2 * tot_ctrl_size;
if (type >= V4L2_CTRL_COMPOUND_TYPES && p_def.p_const)
- sz_extra += elem_size;
+ sz_extra += elem_size * 3;
ctrl = kvzalloc(sizeof(*ctrl) + sz_extra, GFP_KERNEL);
if (ctrl == NULL) {
@@ -2649,6 +2774,13 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
ctrl->p_def.p = ctrl->p_cur.p + tot_ctrl_size;
memcpy(ctrl->p_def.p, p_def.p_const, elem_size);
}
+ if (type >= V4L2_CTRL_COMPOUND_TYPES &&
+ p_min.p_const && p_max.p_const) {
+ ctrl->p_min.p = ctrl->p_cur.p + 2 * tot_ctrl_size;
+ memcpy(ctrl->p_min.p, p_min.p_const, elem_size);
+ ctrl->p_max.p = ctrl->p_cur.p + 3 * tot_ctrl_size;
+ memcpy(ctrl->p_max.p, p_max.p_const, elem_size);
+ }
for (idx = 0; idx < elems; idx++) {
ctrl->type_ops->init(ctrl, idx, ctrl->p_cur);
@@ -2701,7 +2833,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct v4l2_ctrl_handler *hdl,
type, min, max,
is_menu ? cfg->menu_skip_mask : step, def,
cfg->dims, cfg->elem_size,
- flags, qmenu, qmenu_int, cfg->p_def, priv);
+ flags, qmenu, qmenu_int, cfg->p_def, cfg->p_min,
+ cfg->p_max, priv);
if (ctrl)
ctrl->is_private = cfg->is_private;
return ctrl;
@@ -2726,7 +2859,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_std(struct v4l2_ctrl_handler *hdl,
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,
min, max, step, def, NULL, 0,
- flags, NULL, NULL, ptr_null, NULL);
+ flags, NULL, NULL, ptr_null, ptr_null,
+ ptr_null, NULL);
}
EXPORT_SYMBOL(v4l2_ctrl_new_std);
@@ -2759,7 +2893,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl,
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,
0, max, mask, def, NULL, 0,
- flags, qmenu, qmenu_int, ptr_null, NULL);
+ flags, qmenu, qmenu_int, ptr_null, ptr_null,
+ ptr_null, NULL);
}
EXPORT_SYMBOL(v4l2_ctrl_new_std_menu);
@@ -2791,7 +2926,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl,
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,
0, max, mask, def, NULL, 0,
- flags, qmenu, NULL, ptr_null, NULL);
+ flags, qmenu, NULL, ptr_null, ptr_null,
+ ptr_null, NULL);
}
EXPORT_SYMBOL(v4l2_ctrl_new_std_menu_items);
@@ -2799,7 +2935,9 @@ EXPORT_SYMBOL(v4l2_ctrl_new_std_menu_items);
/* Helper function for standard compound controls */
struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl,
const struct v4l2_ctrl_ops *ops, u32 id,
- const union v4l2_ctrl_ptr p_def)
+ const union v4l2_ctrl_ptr p_def,
+ const union v4l2_ctrl_ptr p_min,
+ const union v4l2_ctrl_ptr p_max)
{
const char *name;
enum v4l2_ctrl_type type;
@@ -2813,7 +2951,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl,
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,
min, max, step, def, NULL, 0,
- flags, NULL, NULL, p_def, NULL);
+ flags, NULL, NULL, p_def, p_min, p_max, NULL);
}
EXPORT_SYMBOL(v4l2_ctrl_new_std_compound);
@@ -2837,7 +2975,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct v4l2_ctrl_handler *hdl,
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,
0, max, 0, def, NULL, 0,
- flags, NULL, qmenu_int, ptr_null, NULL);
+ flags, NULL, qmenu_int, ptr_null, ptr_null,
+ ptr_null, NULL);
}
EXPORT_SYMBOL(v4l2_ctrl_new_int_menu);
@@ -3479,8 +3618,8 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
cs->error_idx = i;
if (cs->which &&
- cs->which != V4L2_CTRL_WHICH_DEF_VAL &&
- cs->which != V4L2_CTRL_WHICH_REQUEST_VAL &&
+ (cs->which < V4L2_CTRL_WHICH_DEF_VAL ||
+ cs->which > V4L2_CTRL_WHICH_MAX_VAL) &&
V4L2_CTRL_ID2WHICH(id) != cs->which) {
dprintk(vdev,
"invalid which 0x%x or control id 0x%x\n",
@@ -3578,8 +3717,8 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
whether there are any controls at all. */
static int class_check(struct v4l2_ctrl_handler *hdl, u32 which)
{
- if (which == 0 || which == V4L2_CTRL_WHICH_DEF_VAL ||
- which == V4L2_CTRL_WHICH_REQUEST_VAL)
+ if (which == 0 || (which >= V4L2_CTRL_WHICH_DEF_VAL &&
+ which <= V4L2_CTRL_WHICH_MAX_VAL))
return 0;
return find_ref_lock(hdl, which | 1) ? 0 : -EINVAL;
}
@@ -3593,9 +3732,7 @@ static int v4l2_g_ext_ctrls_common(struct v4l2_ctrl_handler *hdl,
struct v4l2_ctrl_helper *helpers = helper;
int ret;
int i, j;
- bool def_value;
-
- def_value = (cs->which == V4L2_CTRL_WHICH_DEF_VAL);
+ __u32 which = cs->which;
cs->error_idx = cs->count;
cs->which = V4L2_CTRL_ID2WHICH(cs->which);
@@ -3625,18 +3762,31 @@ static int v4l2_g_ext_ctrls_common(struct v4l2_ctrl_handler *hdl,
struct v4l2_ctrl *ctrl);
struct v4l2_ctrl *master;
- ctrl_to_user = def_value ? def_to_user : cur_to_user;
-
if (helpers[i].mref == NULL)
continue;
master = helpers[i].mref->ctrl;
cs->error_idx = i;
+ switch (which) {
+ case V4L2_CTRL_WHICH_DEF_VAL:
+ ctrl_to_user = def_to_user;
+ break;
+ case V4L2_CTRL_WHICH_MIN_VAL:
+ ctrl_to_user = min_to_user;
+ break;
+ case V4L2_CTRL_WHICH_MAX_VAL:
+ ctrl_to_user = max_to_user;
+ break;
+ default:
+ ctrl_to_user = cur_to_user;
+ break;
+ }
+
v4l2_ctrl_lock(master);
/* g_volatile_ctrl will update the new control values */
- if (!def_value &&
+ if (ctrl_to_user == cur_to_user &&
((master->flags & V4L2_CTRL_FLAG_VOLATILE) ||
(master->has_volatiles && !is_cur_manual(master)))) {
for (j = 0; j < master->ncontrols; j++)
@@ -3962,9 +4112,11 @@ static int try_set_ext_ctrls_common(struct v4l2_fh *fh,
cs->error_idx = cs->count;
- /* Default value cannot be changed */
- if (cs->which == V4L2_CTRL_WHICH_DEF_VAL) {
- dprintk(vdev, "%s: cannot change default value\n",
+ /* Default/minimum/maximum values cannot be changed */
+ if (cs->which == V4L2_CTRL_WHICH_DEF_VAL ||
+ cs->which == V4L2_CTRL_WHICH_MIN_VAL ||
+ cs->which == V4L2_CTRL_WHICH_MAX_VAL) {
+ dprintk(vdev, "%s: cannot change default/min/max value\n",
video_device_node_name(vdev));
return -EINVAL;
}
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index aefd62831bc8..6372c75ddc1b 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -122,6 +122,8 @@ struct v4l2_ctrl_ops {
*
* @equal: return true if both values are equal.
* @init: initialize the value.
+ * @minimum: set the value to the minimum value of the control.
+ * @maximum: set the value to the maximum value of the control.
* @log: log the value.
* @validate: validate the value. Return 0 on success and a negative value
* otherwise.
@@ -132,6 +134,10 @@ struct v4l2_ctrl_type_ops {
union v4l2_ctrl_ptr ptr2);
void (*init)(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr);
+ void (*minimum)(const struct v4l2_ctrl *ctrl, u32 idx,
+ union v4l2_ctrl_ptr ptr);
+ void (*maximum)(const struct v4l2_ctrl *ctrl, u32 idx,
+ union v4l2_ctrl_ptr ptr);
void (*log)(const struct v4l2_ctrl *ctrl);
int (*validate)(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr);
@@ -228,6 +234,12 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl *ctrl, void *priv);
* @p_def: The control's default value represented via a union which
* provides a standard way of accessing control types
* through a pointer (for compound controls only).
+ * @p_min: The control's minimum value represented via a union which
+ * provides a standard way of accessing control types
+ * through a pointer (for compound controls only).
+ * @p_max: The control's maximum value represented via a union which
+ * provides a standard way of accessing control types
+ * through a pointer (for compound controls only).
* @p_cur: The control's current value represented via a union which
* provides a standard way of accessing control types
* through a pointer.
@@ -283,6 +295,8 @@ struct v4l2_ctrl {
} cur;
union v4l2_ctrl_ptr p_def;
+ union v4l2_ctrl_ptr p_min;
+ union v4l2_ctrl_ptr p_max;
union v4l2_ctrl_ptr p_new;
union v4l2_ctrl_ptr p_cur;
};
@@ -387,6 +401,8 @@ struct v4l2_ctrl_handler {
* @step: The control's step value for non-menu controls.
* @def: The control's default value.
* @p_def: The control's default value for compound controls.
+ * @p_min: The control's minimum value for compound controls.
+ * @p_max: The control's maximum value for compound controls.
* @dims: The size of each dimension.
* @elem_size: The size in bytes of the control.
* @flags: The control's flags.
@@ -416,6 +432,8 @@ struct v4l2_ctrl_config {
u64 step;
s64 def;
union v4l2_ctrl_ptr p_def;
+ union v4l2_ctrl_ptr p_min;
+ union v4l2_ctrl_ptr p_max;
u32 dims[V4L2_CTRL_MAX_DIMS];
u32 elem_size;
u32 flags;
@@ -685,15 +703,19 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl,
* @ops: The control ops.
* @id: The control ID.
* @p_def: The control's default value.
+ * @p_min: The control's minimum value.
+ * @p_max: The control's maximum value.
*
- * Sames as v4l2_ctrl_new_std(), but with support to compound controls, thanks
- * to the @p_def field.
+ * Same as v4l2_ctrl_new_std(), but with support to compound controls, thanks
+ * to the @p_def/min/max fields.
*
*/
struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl,
const struct v4l2_ctrl_ops *ops,
u32 id,
- const union v4l2_ctrl_ptr p_def);
+ const union v4l2_ctrl_ptr p_def,
+ const union v4l2_ctrl_ptr p_min,
+ const union v4l2_ctrl_ptr p_max);
/**
* v4l2_ctrl_new_int_menu() - Create a new standard V4L2 integer menu control.
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 2b584c69e619..7aa1293ac7bc 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1714,6 +1714,8 @@ struct v4l2_ext_controls {
#define V4L2_CTRL_WHICH_CUR_VAL 0
#define V4L2_CTRL_WHICH_DEF_VAL 0x0f000000
#define V4L2_CTRL_WHICH_REQUEST_VAL 0x0f010000
+#define V4L2_CTRL_WHICH_MIN_VAL 0x0f020000
+#define V4L2_CTRL_WHICH_MAX_VAL 0x0f030000
enum v4l2_ctrl_type {
V4L2_CTRL_TYPE_INTEGER = 1,
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/5] v4l2-controls.h: add V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE
2019-11-19 11:34 [PATCH 0/5] Stateful Encoding: final bits Hans Verkuil
2019-11-19 11:34 ` [PATCH 1/5] v4l2-ctrls: add support for v4l2_fract types Hans Verkuil
2019-11-19 11:34 ` [PATCH 2/5] v4l2-ctrls: add support for V4L2_CTRL_WHICH_MIN/MAX_VAL Hans Verkuil
@ 2019-11-19 11:34 ` Hans Verkuil
2019-11-19 11:34 ` [PATCH 4/5] videodev2.h: add V4L2_BUF_FLAG_TOO_SMALL flag Hans Verkuil
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-11-19 11:34 UTC (permalink / raw)
To: linux-media; +Cc: Michael Tretter, Hans Verkuil
Add a new control to specify the frame rate that a HW encoder
should assume. This is one of the parameters that determines the
compression ratio.
In the past drivers used G/S_PARM for this, but that should be
used to set the speed at which userspace sends frames to the encoder
and it can be used to schedule encoding hardware if there are
multiple encoder cores.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 8 ++++++++
drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++
include/uapi/linux/v4l2-controls.h | 1 +
3 files changed, 13 insertions(+)
diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
index 28313c0f4e7c..3bf11f45e20f 100644
--- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
+++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
@@ -592,6 +592,14 @@ 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_ENC_FRAME_RATE (struct)``
+ The frame rate of the to-be-encoded frames. This information is needed
+ by hardware encoders in order to compress the video stream correctly.
+
+ This control is in frames per second and is represented as a fraction.
+ The struct :c:type:`v4l2_fract` provides the numerator and the denominator
+ of the frame rate.
+
``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 af74609dce0a..863a8fdce99d 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -904,6 +904,7 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE: return "Vertical MV Search Range";
case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER: return "Repeat Sequence Header";
case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME: return "Force Key Frame";
+ case V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE: return "Encoder Frame Rate";
case V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS: return "MPEG-2 Slice Parameters";
case V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION: return "MPEG-2 Quantization Matrices";
case V4L2_CID_MPEG_VIDEO_FWHT_PARAMS: return "FWHT Stateless Parameters";
@@ -1411,6 +1412,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
*type = V4L2_CTRL_TYPE_AREA;
*flags |= V4L2_CTRL_FLAG_READ_ONLY;
break;
+ case V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE:
+ *type = V4L2_CTRL_TYPE_FRACT;
+ break;
default:
*type = V4L2_CTRL_TYPE_INTEGER;
break;
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 5a7bedee2b0e..e4a42e4e293e 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -408,6 +408,7 @@ enum v4l2_mpeg_video_multi_slice_mode {
#define V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE (V4L2_CID_MPEG_BASE+227)
#define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE (V4L2_CID_MPEG_BASE+228)
#define V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME (V4L2_CID_MPEG_BASE+229)
+#define V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE (V4L2_CID_MPEG_BASE+230)
/* CIDs for the MPEG-2 Part 2 (H.262) codec */
#define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL (V4L2_CID_MPEG_BASE+270)
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/5] videodev2.h: add V4L2_BUF_FLAG_TOO_SMALL flag
2019-11-19 11:34 [PATCH 0/5] Stateful Encoding: final bits Hans Verkuil
` (2 preceding siblings ...)
2019-11-19 11:34 ` [PATCH 3/5] v4l2-controls.h: add V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE Hans Verkuil
@ 2019-11-19 11:34 ` Hans Verkuil
2019-11-19 14:36 ` Hans Verkuil
2019-11-19 11:34 ` [PATCH 5/5] media: docs-rst: Document memory-to-memory video encoder interface Hans Verkuil
` (2 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2019-11-19 11:34 UTC (permalink / raw)
To: linux-media; +Cc: Michael Tretter, Hans Verkuil
Stateful encoders need to know if V4L2_BUF_FLAG_ERROR was because
the capture buffer was too small or because there was another
error. Set this flag (always in combination with FLAG_ERROR) to
indicate that the buffer was too small.
A corresponding VB2_BUF_STATE_ERROR_TOO_SMALL vb2 state was added.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
Documentation/media/uapi/v4l/buffer.rst | 9 +++++++++
drivers/media/common/videobuf2/videobuf2-core.c | 12 +++++++++---
drivers/media/common/videobuf2/videobuf2-v4l2.c | 4 ++++
include/media/videobuf2-core.h | 4 ++++
include/uapi/linux/videodev2.h | 2 ++
5 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst
index 9149b57728e5..64a97677ec20 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -540,6 +540,15 @@ Buffer Flags
streaming may continue as normal and the buffer may be reused
normally. Drivers set this flag when the ``VIDIOC_DQBUF`` ioctl is
called.
+ * .. _`V4L2-BUF-FLAG-TOO-SMALL`:
+
+ - ``V4L2_BUF_FLAG_TOO_SMALL``
+ - 0x00080000
+ - When this flag is set, the buffer has been dequeued successfully,
+ but no data was written since the buffer was too small. If this
+ flag is set, then ``V4L2_BUF_FLAG_ERROR`` was also set. This can
+ only happen for capture buffers. Drivers set this flag when
+ the ``VIDIOC_DQBUF`` ioctl is called.
* .. _`V4L2-BUF-FLAG-IN-REQUEST`:
- ``V4L2_BUF_FLAG_IN_REQUEST``
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 4489744fbbd9..187a4589a7bb 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -929,6 +929,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
if (WARN_ON(state != VB2_BUF_STATE_DONE &&
state != VB2_BUF_STATE_ERROR &&
+ state != VB2_BUF_STATE_ERROR_TOO_SMALL &&
state != VB2_BUF_STATE_QUEUED))
state = VB2_BUF_STATE_ERROR;
@@ -1816,6 +1817,9 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
case VB2_BUF_STATE_ERROR:
dprintk(3, "returning done buffer with errors\n");
break;
+ case VB2_BUF_STATE_ERROR_TOO_SMALL:
+ dprintk(3, "returning done buffer that's too small\n");
+ break;
default:
dprintk(1, "invalid buffer state\n");
return -EINVAL;
@@ -2383,8 +2387,9 @@ __poll_t vb2_core_poll(struct vb2_queue *q, struct file *file,
done_entry);
spin_unlock_irqrestore(&q->done_lock, flags);
- if (vb && (vb->state == VB2_BUF_STATE_DONE
- || vb->state == VB2_BUF_STATE_ERROR)) {
+ if (vb && (vb->state == VB2_BUF_STATE_DONE ||
+ vb->state == VB2_BUF_STATE_ERROR ||
+ vb->state == VB2_BUF_STATE_ERROR_TOO_SMALL)) {
return (q->is_output) ?
EPOLLOUT | EPOLLWRNORM :
EPOLLIN | EPOLLRDNORM;
@@ -2812,7 +2817,8 @@ static int vb2_thread(void *data)
break;
try_to_freeze();
- if (vb->state != VB2_BUF_STATE_ERROR)
+ if (vb->state != VB2_BUF_STATE_ERROR &&
+ vb->state != VB2_BUF_STATE_ERROR_TOO_SMALL)
if (threadio->fnc(vb, threadio->priv))
break;
call_void_qop(q, wait_finish, q);
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index e652f4318284..6ac19734e4a2 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -44,6 +44,7 @@ module_param(debug, int, 0644);
/* Flags that are set by us */
#define V4L2_BUFFER_MASK_FLAGS (V4L2_BUF_FLAG_MAPPED | V4L2_BUF_FLAG_QUEUED | \
V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_ERROR | \
+ V4L2_BUF_FLAG_ERROR_TOO_SMALL | \
V4L2_BUF_FLAG_PREPARED | \
V4L2_BUF_FLAG_IN_REQUEST | \
V4L2_BUF_FLAG_REQUEST_FD | \
@@ -546,6 +547,9 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb)
case VB2_BUF_STATE_IN_REQUEST:
b->flags |= V4L2_BUF_FLAG_IN_REQUEST;
break;
+ case VB2_BUF_STATE_ERROR_TOO_SMALL:
+ b->flags |= V4L2_BUF_FLAG_TOO_SMALL;
+ /* fall through */
case VB2_BUF_STATE_ERROR:
b->flags |= V4L2_BUF_FLAG_ERROR;
/* fall through */
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index a2b2208b02da..289c3e090257 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -215,6 +215,9 @@ enum vb2_io_modes {
* @VB2_BUF_STATE_ERROR: same as above, but the operation on the buffer
* has ended with an error, which will be reported
* to the userspace when it is dequeued.
+ * @VB2_BUF_STATE_ERROR_TOO_SMALL: same as above, but the operation on the buffer
+ * has ended with an error because the receiving buffer
+ * is too small.
*/
enum vb2_buffer_state {
VB2_BUF_STATE_DEQUEUED,
@@ -224,6 +227,7 @@ enum vb2_buffer_state {
VB2_BUF_STATE_ACTIVE,
VB2_BUF_STATE_DONE,
VB2_BUF_STATE_ERROR,
+ VB2_BUF_STATE_ERROR_TOO_SMALL,
};
struct vb2_queue;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 7aa1293ac7bc..c7c1179eea65 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1064,6 +1064,8 @@ static inline __u64 v4l2_timeval_to_ns(const struct timeval *tv)
#define V4L2_BUF_FLAG_TSTAMP_SRC_MASK 0x00070000
#define V4L2_BUF_FLAG_TSTAMP_SRC_EOF 0x00000000
#define V4L2_BUF_FLAG_TSTAMP_SRC_SOE 0x00010000
+/* mem2mem encoder */
+#define V4L2_BUF_FLAG_TOO_SMALL 0x00080000
/* mem2mem encoder/decoder */
#define V4L2_BUF_FLAG_LAST 0x00100000
/* request_fd is valid */
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/5] media: docs-rst: Document memory-to-memory video encoder interface
2019-11-19 11:34 [PATCH 0/5] Stateful Encoding: final bits Hans Verkuil
` (3 preceding siblings ...)
2019-11-19 11:34 ` [PATCH 4/5] videodev2.h: add V4L2_BUF_FLAG_TOO_SMALL flag Hans Verkuil
@ 2019-11-19 11:34 ` Hans Verkuil
2019-11-19 16:15 ` [PATCH 0/5] Stateful Encoding: final bits Michael Tretter
2019-12-20 13:47 ` Michael Tretter
6 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-11-19 11:34 UTC (permalink / raw)
To: linux-media; +Cc: Michael Tretter, Tomasz Figa, Hans Verkuil
From: Tomasz Figa <tfiga@chromium.org>
Due to complexity of the video encoding process, the V4L2 drivers of
stateful encoder hardware require specific sequences of V4L2 API calls
to be followed. These include capability enumeration, initialization,
encoding, encode parameters change, drain and reset.
Specifics of the above have been discussed during Media Workshops at
LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
Conference Europe 2014 in Düsseldorf. The de facto Codec API that
originated at those events was later implemented by the drivers we already
have merged in mainline, such as s5p-mfc or coda.
The only thing missing was the real specification included as a part of
Linux Media documentation. Fix it now and document the encoder part of
the Codec API.
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
Documentation/media/uapi/v4l/dev-encoder.rst | 608 ++++++++++++++++++
Documentation/media/uapi/v4l/dev-mem2mem.rst | 1 +
Documentation/media/uapi/v4l/pixfmt-v4l2.rst | 5 +
Documentation/media/uapi/v4l/v4l2.rst | 2 +
.../media/uapi/v4l/vidioc-encoder-cmd.rst | 51 +-
5 files changed, 647 insertions(+), 20 deletions(-)
create mode 100644 Documentation/media/uapi/v4l/dev-encoder.rst
diff --git a/Documentation/media/uapi/v4l/dev-encoder.rst b/Documentation/media/uapi/v4l/dev-encoder.rst
new file mode 100644
index 000000000000..e9d29d6de67a
--- /dev/null
+++ b/Documentation/media/uapi/v4l/dev-encoder.rst
@@ -0,0 +1,608 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. _encoder:
+
+*************************************************
+Memory-to-Memory Stateful Video Encoder Interface
+*************************************************
+
+A stateful video encoder takes raw video frames in display order and encodes
+them into a bytestream. It generates complete chunks of the bytestream, including
+all metadata, headers, etc. The resulting bytestream does not require any
+further post-processing by the client.
+
+Performing software stream processing, header generation etc. in the driver
+in order to support this interface is strongly discouraged. In case such
+operations are needed, use of the Stateless Video Encoder Interface (in
+development) is strongly advised.
+
+Conventions and Notations Used in This Document
+===============================================
+
+1. The general V4L2 API rules apply if not specified in this document
+ otherwise.
+
+2. The meaning of words "must", "may", "should", etc. is as per `RFC
+ 2119 <https://tools.ietf.org/html/rfc2119>`_.
+
+3. All steps not marked "optional" are required.
+
+4. :c:func:`VIDIOC_G_EXT_CTRLS` and :c:func:`VIDIOC_S_EXT_CTRLS` may be used
+ interchangeably with :c:func:`VIDIOC_G_CTRL` and :c:func:`VIDIOC_S_CTRL`,
+ unless specified otherwise.
+
+5. Single-planar API (see :ref:`planar-apis`) and applicable structures may be
+ used interchangeably with multi-planar API, unless specified otherwise,
+ depending on encoder capabilities and following the general V4L2 guidelines.
+
+6. i = [a..b]: sequence of integers from a to b, inclusive, i.e. i =
+ [0..2]: i = 0, 1, 2.
+
+7. Given an ``OUTPUT`` buffer A, then A’ represents a buffer on the ``CAPTURE``
+ queue containing data that resulted from processing buffer A.
+
+Glossary
+========
+
+Refer to :ref:`decoder-glossary`.
+
+State Machine
+=============
+
+.. kernel-render:: DOT
+ :alt: DOT digraph of encoder state machine
+ :caption: Encoder State Machine
+
+ digraph encoder_state_machine {
+ node [shape = doublecircle, label="Encoding"] Encoding;
+
+ node [shape = circle, label="Initialization"] Initialization;
+ node [shape = circle, label="Stopped"] Stopped;
+ node [shape = circle, label="Drain"] Drain;
+ node [shape = circle, label="Reset"] Reset;
+
+ node [shape = point]; qi
+ qi -> Initialization [ label = "open()" ];
+
+ Initialization -> Encoding [ label = "Both queues streaming" ];
+
+ Encoding -> Drain [ label = "V4L2_ENC_CMD_STOP" ];
+ Encoding -> Reset [ label = "VIDIOC_STREAMOFF(CAPTURE)" ];
+ Encoding -> Stopped [ label = "VIDIOC_STREAMOFF(OUTPUT)" ];
+ Encoding -> Encoding;
+
+ Drain -> Stopped [ label = "All CAPTURE\nbuffers dequeued\nor\nVIDIOC_STREAMOFF(OUTPUT)" ];
+ Drain -> Reset [ label = "VIDIOC_STREAMOFF(CAPTURE)" ];
+
+ Reset -> Encoding [ label = "VIDIOC_STREAMON(CAPTURE)" ];
+ Reset -> Initialization [ label = "VIDIOC_REQBUFS(OUTPUT, 0)" ];
+
+ Stopped -> Encoding [ label = "V4L2_ENC_CMD_START\nor\nVIDIOC_STREAMON(OUTPUT)" ];
+ Stopped -> Reset [ label = "VIDIOC_STREAMOFF(CAPTURE)" ];
+ }
+
+Querying Capabilities
+=====================
+
+1. To enumerate the set of coded formats supported by the encoder, the
+ client may call :c:func:`VIDIOC_ENUM_FMT` on ``CAPTURE``.
+
+ * The full set of supported formats will be returned, regardless of the
+ format set on ``OUTPUT``.
+
+2. To enumerate the set of supported raw formats, the client may call
+ :c:func:`VIDIOC_ENUM_FMT` on ``OUTPUT``.
+
+ * Only the formats supported for the format currently active on ``CAPTURE``
+ will be returned.
+
+ * In order to enumerate raw formats supported by a given coded format,
+ the client must first set that coded format on ``CAPTURE`` and then
+ enumerate the formats on ``OUTPUT``.
+
+3. The client may use :c:func:`VIDIOC_ENUM_FRAMESIZES` to detect supported
+ resolutions for a given format, passing desired pixel format in
+ :c:type:`v4l2_frmsizeenum` ``pixel_format``.
+
+ * Values returned by :c:func:`VIDIOC_ENUM_FRAMESIZES` for a coded pixel
+ format will include all possible coded resolutions supported by the
+ encoder for given coded pixel format.
+
+ * Values returned by :c:func:`VIDIOC_ENUM_FRAMESIZES` for a raw pixel format
+ will include all possible frame buffer resolutions supported by the
+ encoder for given raw pixel format and coded format currently set on
+ ``CAPTURE``.
+
+4. Supported profiles and levels for the coded format currently set on
+ ``CAPTURE``, if applicable, may be queried using their respective controls
+ via :c:func:`VIDIOC_QUERYCTRL`.
+
+5. Any additional encoder capabilities may be discovered by querying
+ their respective controls.
+
+Initialization
+==============
+
+1. Set the coded format on the ``CAPTURE`` queue via :c:func:`VIDIOC_S_FMT`
+
+ * **Required fields:**
+
+ ``type``
+ a ``V4L2_BUF_TYPE_*`` enum appropriate for ``CAPTURE``.
+
+ ``pixelformat``
+ the coded format to be produced.
+
+ ``sizeimage``
+ desired size of ``CAPTURE`` buffers; the encoder may adjust it to
+ match hardware requirements.
+
+ ``width``, ``height``
+ ignored (read-only).
+
+ other fields
+ follow standard semantics.
+
+ * **Return fields:**
+
+ ``sizeimage``
+ adjusted size of ``CAPTURE`` buffers.
+
+ ``width``, ``height``
+ the coded size selected by the encoder based on current state, e.g.
+ ``OUTPUT`` format, selection rectangles, etc. (read-only).
+
+ .. important::
+
+ Changing the ``CAPTURE`` format may change the currently set ``OUTPUT``
+ format. How the new ``OUTPUT`` format is determined is up to the encoder
+ and the client must ensure it matches its needs afterwards.
+
+2. **Optional.** Enumerate supported ``OUTPUT`` formats (raw formats for
+ source) for the selected coded format via :c:func:`VIDIOC_ENUM_FMT`.
+
+ * **Required fields:**
+
+ ``type``
+ a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``.
+
+ other fields
+ follow standard semantics.
+
+ * **Return fields:**
+
+ ``pixelformat``
+ raw format supported for the coded format currently selected on
+ the ``CAPTURE`` queue.
+
+ other fields
+ follow standard semantics.
+
+3. Set the raw source format on the ``OUTPUT`` queue via
+ :c:func:`VIDIOC_S_FMT`.
+
+ * **Required fields:**
+
+ ``type``
+ a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``.
+
+ ``pixelformat``
+ raw format of the source.
+
+ ``width``, ``height``
+ source resolution.
+
+ other fields
+ follow standard semantics.
+
+ * **Return fields:**
+
+ ``width``, ``height``
+ may be adjusted to match encoder minimums, maximums and alignment
+ requirements, as required by the currently selected formats, as
+ reported by :c:func:`VIDIOC_ENUM_FRAMESIZES`.
+
+ other fields
+ follow standard semantics.
+
+ * Setting the ``OUTPUT`` format will reset the selection rectangles to their
+ default values, based on the new resolution, as described in the next
+ step.
+
+4. **Optional.** Set the visible resolution for the stream metadata via
+ :c:func:`VIDIOC_S_SELECTION` on the ``OUTPUT`` queue if it is desired
+ to be different than the full OUTPUT resolution.
+
+ * **Required fields:**
+
+ ``type``
+ a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``.
+
+ ``target``
+ set to ``V4L2_SEL_TGT_CROP``.
+
+ ``r.left``, ``r.top``, ``r.width``, ``r.height``
+ visible rectangle; this must fit within the `V4L2_SEL_TGT_CROP_BOUNDS`
+ rectangle and may be subject to adjustment to match codec and
+ hardware constraints.
+
+ * **Return fields:**
+
+ ``r.left``, ``r.top``, ``r.width``, ``r.height``
+ visible rectangle adjusted by the encoder.
+
+ * The following selection targets are supported on ``OUTPUT``:
+
+ ``V4L2_SEL_TGT_CROP_BOUNDS``
+ equal to the full source frame, matching the active ``OUTPUT``
+ format.
+
+ ``V4L2_SEL_TGT_CROP_DEFAULT``
+ equal to ``V4L2_SEL_TGT_CROP_BOUNDS``.
+
+ ``V4L2_SEL_TGT_CROP``
+ rectangle within the source buffer to be encoded into the
+ ``CAPTURE`` stream; defaults to ``V4L2_SEL_TGT_CROP_DEFAULT``.
+
+ .. note::
+
+ A common use case for this selection target is encoding a source
+ video with a resolution that is not a multiple of a macroblock,
+ e.g. the common 1920x1080 resolution may require the source
+ buffers to be aligned to 1920x1088 for codecs with 16x16 macroblock
+ size. To avoid encoding the padding, the client needs to explicitly
+ configure this selection target to 1920x1080.
+
+ .. warning::
+
+ The encoder may adjust the crop/compose rectangles to the nearest
+ supported ones to meet codec and hardware requirements. The client needs
+ to check the adjusted rectangle returned by :c:func:`VIDIOC_S_SELECTION`.
+
+5. Allocate buffers for both ``OUTPUT`` and ``CAPTURE`` via
+ :c:func:`VIDIOC_REQBUFS`. This may be performed in any order.
+
+ * **Required fields:**
+
+ ``count``
+ requested number of buffers to allocate; greater than zero.
+
+ ``type``
+ a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT`` or
+ ``CAPTURE``.
+
+ other fields
+ follow standard semantics.
+
+ * **Return fields:**
+
+ ``count``
+ actual number of buffers allocated.
+
+ .. warning::
+
+ The actual number of allocated buffers may differ from the ``count``
+ given. The client must check the updated value of ``count`` after the
+ call returns.
+
+ .. note::
+
+ To allocate more than the minimum number of OUTPUT buffers (for pipeline
+ depth), the client may query the ``V4L2_CID_MIN_BUFFERS_FOR_OUTPUT``
+ control to get the minimum number of buffers required, and pass the
+ obtained value plus the number of additional buffers needed in the
+ ``count`` field to :c:func:`VIDIOC_REQBUFS`.
+
+ Alternatively, :c:func:`VIDIOC_CREATE_BUFS` can be used to have more
+ control over buffer allocation.
+
+ * **Required fields:**
+
+ ``count``
+ requested number of buffers to allocate; greater than zero.
+
+ ``type``
+ a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``.
+
+ other fields
+ follow standard semantics.
+
+ * **Return fields:**
+
+ ``count``
+ adjusted to the number of allocated buffers.
+
+6. Begin streaming on both ``OUTPUT`` and ``CAPTURE`` queues via
+ :c:func:`VIDIOC_STREAMON`. This may be performed in any order. The actual
+ encoding process starts when both queues start streaming.
+
+.. note::
+
+ If the client stops the ``CAPTURE`` queue during the encode process and then
+ restarts it again, the encoder will begin generating a stream independent
+ from the stream generated before the stop. The exact constraints depend
+ on the coded format, but may include the following implications:
+
+ * encoded frames produced after the restart must not reference any
+ frames produced before the stop, e.g. no long term references for
+ H.264/HEVC,
+
+ * any headers that must be included in a standalone stream must be
+ produced again, e.g. SPS and PPS for H.264/HEVC.
+
+Encoding
+========
+
+This state is reached after the `Initialization` sequence finishes
+successfully. In this state, the client queues and dequeues buffers to both
+queues via :c:func:`VIDIOC_QBUF` and :c:func:`VIDIOC_DQBUF`, following the
+standard semantics.
+
+The content of encoded ``CAPTURE`` buffers depends on the active coded pixel
+format and may be affected by codec-specific extended controls, as stated
+in the documentation of each format.
+
+Both queues operate independently, following standard behavior of V4L2 buffer
+queues and memory-to-memory devices. In addition, the order of encoded frames
+dequeued from the ``CAPTURE`` queue may differ from the order of queuing raw
+frames to the ``OUTPUT`` queue, due to properties of the selected coded format,
+e.g. frame reordering.
+
+The client must not assume any direct relationship between ``CAPTURE`` and
+``OUTPUT`` buffers and any specific timing of buffers becoming
+available to dequeue. Specifically:
+
+* a buffer queued to ``OUTPUT`` may result in more than one buffer produced on
+ ``CAPTURE`` (if returning an encoded frame allowed the encoder to return a
+ frame that preceded it in display, but succeeded it in the decode order),
+
+* a buffer queued to ``OUTPUT`` may result in a buffer being produced on
+ ``CAPTURE`` later into encode process, and/or after processing further
+ ``OUTPUT`` buffers, or be returned out of order, e.g. if display
+ reordering is used,
+
+* buffers may become available on the ``CAPTURE`` queue without additional
+ buffers queued to ``OUTPUT`` (e.g. during drain or ``EOS``), because of the
+ ``OUTPUT`` buffers queued in the past whose decoding results are only
+ available at later time, due to specifics of the decoding process,
+
+* buffers queued to ``OUTPUT`` may not become available to dequeue instantly
+ after being encoded into a corresponding ``CAPTURE`` buffer, e.g. if the
+ encoder needs to use the frame as a reference for encoding further frames.
+
+.. note::
+
+ To allow matching encoded ``CAPTURE`` buffers with ``OUTPUT`` buffers they
+ originated from, the client can set the ``timestamp`` field of the
+ :c:type:`v4l2_buffer` struct when queuing an ``OUTPUT`` buffer. The
+ ``CAPTURE`` buffer(s), which resulted from encoding that ``OUTPUT`` buffer
+ will have their ``timestamp`` field set to the same value when dequeued.
+
+ In addition to the straightforward case of one ``OUTPUT`` buffer producing
+ one ``CAPTURE`` buffer, the following cases are defined:
+
+ * one ``OUTPUT`` buffer generates multiple ``CAPTURE`` buffers: the same
+ ``OUTPUT`` timestamp will be copied to multiple ``CAPTURE`` buffers,
+
+ * the encoding order differs from the presentation order (i.e. the
+ ``CAPTURE`` buffers are out-of-order compared to the ``OUTPUT`` buffers):
+ ``CAPTURE`` timestamps will not retain the order of ``OUTPUT`` timestamps.
+
+.. note::
+
+ To let the client distinguish between frame types (keyframes, intermediate
+ frames; the exact list of types depends on the coded format), the
+ ``CAPTURE`` buffers will have corresponding flag bits set in their
+ :c:type:`v4l2_buffer` struct when dequeued. See the documentation of
+ :c:type:`v4l2_buffer` and each coded pixel format for exact list of flags
+ and their meanings.
+
+Should an encoding error occur, it will be reported to the client with the level
+of details depending on the encoder capabilities. Specifically:
+
+* the CAPTURE buffer (if any) that contains the results of the failed encode
+ operation will be returned with the V4L2_BUF_FLAG_ERROR flag set,
+
+* if the encoder is able to precisely report the OUTPUT buffer(s) that triggered
+ the error, such buffer(s) will be returned with the V4L2_BUF_FLAG_ERROR flag
+ set.
+
+In case of a fatal failure that does not allow the encoding to continue, any
+further operations on corresponding encoder file handle will return the -EIO
+error code. The client may close the file handle and open a new one, or
+alternatively reinitialize the instance by stopping streaming on both queues,
+releasing all buffers and performing the Initialization sequence again.
+
+Encoding Parameter Changes
+==========================
+
+The client is allowed to use :c:func:`VIDIOC_S_CTRL` to change encoder
+parameters at any time. The availability of parameters is encoder-specific
+and the client must query the encoder to find the set of available controls.
+
+The ability to change each parameter during encoding is encoder-specific, as
+per the standard semantics of the V4L2 control interface. The client may
+attempt to set a control during encoding and if the operation fails with the
+-EBUSY error code, the ``CAPTURE`` queue needs to be stopped for the
+configuration change to be allowed. To do this, it may follow the `Drain`
+sequence to avoid losing the already queued/encoded frames.
+
+The timing of parameter updates is encoder-specific, as per the standard
+semantics of the V4L2 control interface. If the client needs to apply the
+parameters exactly at specific frame, using the Request API
+(:ref:`media-request-api`) should be considered, if supported by the encoder.
+
+Drain
+=====
+
+To ensure that all the queued ``OUTPUT`` buffers have been processed and the
+related ``CAPTURE`` buffers are given to the client, the client must follow the
+drain sequence described below. After the drain sequence ends, the client has
+received all encoded frames for all ``OUTPUT`` buffers queued before the
+sequence was started.
+
+1. Begin the drain sequence by issuing :c:func:`VIDIOC_ENCODER_CMD`.
+
+ * **Required fields:**
+
+ ``cmd``
+ set to ``V4L2_ENC_CMD_STOP``.
+
+ ``flags``
+ set to 0.
+
+ ``pts``
+ set to 0.
+
+ .. warning::
+
+ The sequence can be only initiated if both ``OUTPUT`` and ``CAPTURE``
+ queues are streaming. For compatibility reasons, the call to
+ :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is
+ not streaming, but at the same time it will not initiate the `Drain`
+ sequence and so the steps described below would not be applicable.
+
+2. Any ``OUTPUT`` buffers queued by the client before the
+ :c:func:`VIDIOC_ENCODER_CMD` was issued will be processed and encoded as
+ normal. The client must continue to handle both queues independently,
+ similarly to normal encode operation. This includes:
+
+ * queuing and dequeuing ``CAPTURE`` buffers, until a buffer marked with the
+ ``V4L2_BUF_FLAG_LAST`` flag is dequeued,
+
+ .. warning::
+
+ The last buffer may be empty (with :c:type:`v4l2_buffer`
+ ``bytesused`` = 0) and in that case it must be ignored by the client,
+ as it does not contain an encoded frame.
+
+ .. note::
+
+ Any attempt to dequeue more ``CAPTURE`` buffers beyond the buffer
+ marked with ``V4L2_BUF_FLAG_LAST`` will result in a -EPIPE error from
+ :c:func:`VIDIOC_DQBUF`.
+
+ * dequeuing processed ``OUTPUT`` buffers, until all the buffers queued
+ before the ``V4L2_ENC_CMD_STOP`` command are dequeued,
+
+ * dequeuing the ``V4L2_EVENT_EOS`` event, if the client subscribes to it.
+
+ .. note::
+
+ For backwards compatibility, the encoder will signal a ``V4L2_EVENT_EOS``
+ event when the last frame has been encoded and all frames are ready to be
+ dequeued. It is deprecated behavior and the client must not rely on it.
+ The ``V4L2_BUF_FLAG_LAST`` buffer flag should be used instead.
+
+3. Once all ``OUTPUT`` buffers queued before the ``V4L2_ENC_CMD_STOP`` call are
+ dequeued and the last ``CAPTURE`` buffer is dequeued, the encoder is stopped
+ and it will accept, but not process any newly queued ``OUTPUT`` buffers
+ until the client issues any of the following operations:
+
+ * ``V4L2_ENC_CMD_START`` - the encoder will not be reset and will resume
+ operation normally, with all the state from before the drain,
+
+ * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
+ ``CAPTURE`` queue - the encoder will be reset (see the `Reset` sequence)
+ and then resume encoding,
+
+ * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
+ ``OUTPUT`` queue - the encoder will resume operation normally, however any
+ source frames queued to the ``OUTPUT`` queue between ``V4L2_ENC_CMD_STOP``
+ and :c:func:`VIDIOC_STREAMOFF` will be discarded.
+
+.. note::
+
+ Once the drain sequence is initiated, the client needs to drive it to
+ completion, as described by the steps above, unless it aborts the process by
+ issuing :c:func:`VIDIOC_STREAMOFF` on any of the ``OUTPUT`` or ``CAPTURE``
+ queues. The client is not allowed to issue ``V4L2_ENC_CMD_START`` or
+ ``V4L2_ENC_CMD_STOP`` again while the drain sequence is in progress and they
+ will fail with -EBUSY error code if attempted.
+
+ For reference, handling of various corner cases is described below:
+
+ * In case of no buffer in the ``OUTPUT`` queue at the time the
+ ``V4L2_ENC_CMD_STOP`` command was issued, the drain sequence completes
+ immediately and the encoder returns an empty ``CAPTURE`` buffer with the
+ ``V4L2_BUF_FLAG_LAST`` flag set.
+
+ * In case of no buffer in the ``CAPTURE`` queue at the time the drain
+ sequence completes, the next time the client queues a ``CAPTURE`` buffer
+ it is returned at once as an empty buffer with the ``V4L2_BUF_FLAG_LAST``
+ flag set.
+
+ * If :c:func:`VIDIOC_STREAMOFF` is called on the ``CAPTURE`` queue in the
+ middle of the drain sequence, the drain sequence is canceled and all
+ ``CAPTURE`` buffers are implicitly returned to the client.
+
+ * If :c:func:`VIDIOC_STREAMOFF` is called on the ``OUTPUT`` queue in the
+ middle of the drain sequence, the drain sequence completes immediately and
+ next ``CAPTURE`` buffer will be returned empty with the
+ ``V4L2_BUF_FLAG_LAST`` flag set.
+
+ Although mandatory, the availability of encoder commands may be queried
+ using :c:func:`VIDIOC_TRY_ENCODER_CMD`.
+
+Reset
+=====
+
+The client may want to request the encoder to reinitialize the encoding, so
+that the following stream data becomes independent from the stream data
+generated before. Depending on the coded format, that may imply that:
+
+* encoded frames produced after the restart must not reference any frames
+ produced before the stop, e.g. no long term references for H.264/HEVC,
+
+* any headers that must be included in a standalone stream must be produced
+ again, e.g. SPS and PPS for H.264/HEVC.
+
+This can be achieved by performing the reset sequence.
+
+1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
+ and respective buffers are dequeued.
+
+2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
+ will return all currently queued ``CAPTURE`` buffers to the client, without
+ valid frame data.
+
+3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
+ continue with regular encoding sequence. The encoded frames produced into
+ ``CAPTURE`` buffers from now on will contain a standalone stream that can be
+ decoded without the need for frames encoded before the reset sequence,
+ starting at the first ``OUTPUT`` buffer queued after issuing the
+ `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
+
+This sequence may be also used to change encoding parameters for encoders
+without the ability to change the parameters on the fly.
+
+Commit Points
+=============
+
+Setting formats and allocating buffers triggers changes in the behavior of the
+encoder.
+
+1. Setting the format on the ``CAPTURE`` queue may change the set of formats
+ supported/advertised on the ``OUTPUT`` queue. In particular, it also means
+ that the ``OUTPUT`` format may be reset and the client must not rely on the
+ previously set format being preserved.
+
+2. Enumerating formats on the ``OUTPUT`` queue always returns only formats
+ supported for the current ``CAPTURE`` format.
+
+3. Setting the format on the ``OUTPUT`` queue does not change the list of
+ formats available on the ``CAPTURE`` queue. An attempt to set the ``OUTPUT``
+ format that is not supported for the currently selected ``CAPTURE`` format
+ will result in the encoder adjusting the requested ``OUTPUT`` format to a
+ supported one.
+
+4. Enumerating formats on the ``CAPTURE`` queue always returns the full set of
+ supported coded formats, irrespective of the current ``OUTPUT`` format.
+
+5. While buffers are allocated on any of the ``OUTPUT`` or ``CAPTURE`` queues,
+ the client must not change the format on the ``CAPTURE`` queue. Drivers will
+ return the -EBUSY error code for any such format change attempt.
+
+To summarize, setting formats and allocation must always start with the
+``CAPTURE`` queue and the ``CAPTURE`` queue is the master that governs the
+set of supported formats for the ``OUTPUT`` queue.
diff --git a/Documentation/media/uapi/v4l/dev-mem2mem.rst b/Documentation/media/uapi/v4l/dev-mem2mem.rst
index 70953958cee6..43f14ccb675e 100644
--- a/Documentation/media/uapi/v4l/dev-mem2mem.rst
+++ b/Documentation/media/uapi/v4l/dev-mem2mem.rst
@@ -46,4 +46,5 @@ devices are given in the following sections.
:maxdepth: 1
dev-decoder
+ dev-encoder
dev-stateless-decoder
diff --git a/Documentation/media/uapi/v4l/pixfmt-v4l2.rst b/Documentation/media/uapi/v4l/pixfmt-v4l2.rst
index a8321c348bf8..b7ccc613a382 100644
--- a/Documentation/media/uapi/v4l/pixfmt-v4l2.rst
+++ b/Documentation/media/uapi/v4l/pixfmt-v4l2.rst
@@ -44,6 +44,11 @@ Single-planar format structure
inside the stream, when fed to a stateful mem2mem decoder, the fields
may be zero to rely on the decoder to detect the right values. For more
details see :ref:`decoder` and format descriptions.
+
+ For compressed formats on the CAPTURE side of a stateful mem2mem
+ encoder, the fields must be zero, since the coded size is expected to
+ be calculated internally by the encoder itself, based on the OUTPUT
+ side. For more details see :ref:`encoder` and format descriptions.
* - __u32
- ``pixelformat``
- The pixel format or type of compression, set by the application.
diff --git a/Documentation/media/uapi/v4l/v4l2.rst b/Documentation/media/uapi/v4l/v4l2.rst
index 97015b9b40b8..e236ea23481b 100644
--- a/Documentation/media/uapi/v4l/v4l2.rst
+++ b/Documentation/media/uapi/v4l/v4l2.rst
@@ -63,6 +63,7 @@ Authors, in alphabetical order:
- Figa, Tomasz <tfiga@chromium.org>
- Documented the memory-to-memory decoder interface.
+ - Documented the memory-to-memory encoder interface.
- H Schimek, Michael <mschimek@gmx.at>
@@ -75,6 +76,7 @@ Authors, in alphabetical order:
- Osciak, Pawel <posciak@chromium.org>
- Documented the memory-to-memory decoder interface.
+ - Documented the memory-to-memory encoder interface.
- Osciak, Pawel <pawel@osciak.com>
diff --git a/Documentation/media/uapi/v4l/vidioc-encoder-cmd.rst b/Documentation/media/uapi/v4l/vidioc-encoder-cmd.rst
index c313ca8b8cb5..88281e707476 100644
--- a/Documentation/media/uapi/v4l/vidioc-encoder-cmd.rst
+++ b/Documentation/media/uapi/v4l/vidioc-encoder-cmd.rst
@@ -51,25 +51,26 @@ To send a command applications must initialize all fields of a struct
``VIDIOC_ENCODER_CMD`` or ``VIDIOC_TRY_ENCODER_CMD`` with a pointer to
this structure.
-The ``cmd`` field must contain the command code. The ``flags`` field is
-currently only used by the STOP command and contains one bit: If the
-``V4L2_ENC_CMD_STOP_AT_GOP_END`` flag is set, encoding will continue
-until the end of the current *Group Of Pictures*, otherwise it will stop
-immediately.
+The ``cmd`` field must contain the command code. Some commands use the
+``flags`` field for additional information.
-A :ref:`read() <func-read>` or :ref:`VIDIOC_STREAMON <VIDIOC_STREAMON>`
-call sends an implicit START command to the encoder if it has not been
-started yet. After a STOP command, :ref:`read() <func-read>` calls will read
+After a STOP command, :ref:`read() <func-read>` calls will read
the remaining data buffered by the driver. When the buffer is empty,
:ref:`read() <func-read>` will return zero and the next :ref:`read() <func-read>`
call will restart the encoder.
+A :ref:`read() <func-read>` or :ref:`VIDIOC_STREAMON <VIDIOC_STREAMON>`
+call sends an implicit START command to the encoder if it has not been
+started yet. Applies to both queues of mem2mem encoders.
+
A :ref:`close() <func-close>` or :ref:`VIDIOC_STREAMOFF <VIDIOC_STREAMON>`
call of a streaming file descriptor sends an implicit immediate STOP to
-the encoder, and all buffered data is discarded.
+the encoder, and all buffered data is discarded. Applies to both queues of
+mem2mem encoders.
These ioctls are optional, not all drivers may support them. They were
-introduced in Linux 2.6.21.
+introduced in Linux 2.6.21. They are, however, mandatory for stateful mem2mem
+encoders (as further documented in :ref:`encoder`).
.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{8.7cm}|
@@ -109,21 +110,24 @@ introduced in Linux 2.6.21.
- 0
- Start the encoder. When the encoder is already running or paused,
this command does nothing. No flags are defined for this command.
+
+ For a device implementing the :ref:`encoder`, once the drain sequence
+ is initiated with the ``V4L2_ENC_CMD_STOP`` command, it must be driven
+ to completion before this command can be invoked. Any attempt to
+ invoke the command while the drain sequence is in progress will trigger
+ an ``EBUSY`` error code. See :ref:`encoder` for more details.
* - ``V4L2_ENC_CMD_STOP``
- 1
- Stop the encoder. When the ``V4L2_ENC_CMD_STOP_AT_GOP_END`` flag
is set, encoding will continue until the end of the current *Group
Of Pictures*, otherwise encoding will stop immediately. When the
- encoder is already stopped, this command does nothing. mem2mem
- encoders will send a ``V4L2_EVENT_EOS`` event when the last frame
- has been encoded and all frames are ready to be dequeued and will
- set the ``V4L2_BUF_FLAG_LAST`` buffer flag on the last buffer of
- the capture queue to indicate there will be no new buffers
- produced to dequeue. This buffer may be empty, indicated by the
- driver setting the ``bytesused`` field to 0. Once the
- ``V4L2_BUF_FLAG_LAST`` flag was set, the
- :ref:`VIDIOC_DQBUF <VIDIOC_QBUF>` ioctl will not block anymore,
- but return an ``EPIPE`` error code.
+ encoder is already stopped, this command does nothing.
+
+ For a device implementing the :ref:`encoder`, the command will initiate
+ the drain sequence as documented in :ref:`encoder`. No flags or other
+ arguments are accepted in this case. Any attempt to invoke the command
+ again before the sequence completes will trigger an ``EBUSY`` error
+ code.
* - ``V4L2_ENC_CMD_PAUSE``
- 2
- Pause the encoder. When the encoder has not been started yet, the
@@ -152,6 +156,8 @@ introduced in Linux 2.6.21.
- Stop encoding at the end of the current *Group Of Pictures*,
rather than immediately.
+ Does not apply to :ref:`encoder`.
+
Return Value
============
@@ -160,6 +166,11 @@ On success 0 is returned, on error -1 and the ``errno`` variable is set
appropriately. The generic error codes are described at the
:ref:`Generic Error Codes <gen-errors>` chapter.
+EBUSY
+ A drain sequence of a device implementing the :ref:`encoder` is still in
+ progress. It is not allowed to issue another encoder command until it
+ completes.
+
EINVAL
The ``cmd`` field is invalid.
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 4/5] videodev2.h: add V4L2_BUF_FLAG_TOO_SMALL flag
2019-11-19 11:34 ` [PATCH 4/5] videodev2.h: add V4L2_BUF_FLAG_TOO_SMALL flag Hans Verkuil
@ 2019-11-19 14:36 ` Hans Verkuil
0 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-11-19 14:36 UTC (permalink / raw)
To: linux-media; +Cc: Michael Tretter
On 11/19/19 12:34 PM, Hans Verkuil wrote:
> Stateful encoders need to know if V4L2_BUF_FLAG_ERROR was because
> the capture buffer was too small or because there was another
> error. Set this flag (always in combination with FLAG_ERROR) to
> indicate that the buffer was too small.
>
> A corresponding VB2_BUF_STATE_ERROR_TOO_SMALL vb2 state was added.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
> Documentation/media/uapi/v4l/buffer.rst | 9 +++++++++
> drivers/media/common/videobuf2/videobuf2-core.c | 12 +++++++++---
> drivers/media/common/videobuf2/videobuf2-v4l2.c | 4 ++++
> include/media/videobuf2-core.h | 4 ++++
> include/uapi/linux/videodev2.h | 2 ++
> 5 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst
> index 9149b57728e5..64a97677ec20 100644
> --- a/Documentation/media/uapi/v4l/buffer.rst
> +++ b/Documentation/media/uapi/v4l/buffer.rst
> @@ -540,6 +540,15 @@ Buffer Flags
> streaming may continue as normal and the buffer may be reused
> normally. Drivers set this flag when the ``VIDIOC_DQBUF`` ioctl is
> called.
> + * .. _`V4L2-BUF-FLAG-TOO-SMALL`:
> +
> + - ``V4L2_BUF_FLAG_TOO_SMALL``
> + - 0x00080000
> + - When this flag is set, the buffer has been dequeued successfully,
> + but no data was written since the buffer was too small. If this
> + flag is set, then ``V4L2_BUF_FLAG_ERROR`` was also set. This can
> + only happen for capture buffers. Drivers set this flag when
> + the ``VIDIOC_DQBUF`` ioctl is called.
> * .. _`V4L2-BUF-FLAG-IN-REQUEST`:
>
> - ``V4L2_BUF_FLAG_IN_REQUEST``
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index 4489744fbbd9..187a4589a7bb 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -929,6 +929,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
>
> if (WARN_ON(state != VB2_BUF_STATE_DONE &&
> state != VB2_BUF_STATE_ERROR &&
> + state != VB2_BUF_STATE_ERROR_TOO_SMALL &&
> state != VB2_BUF_STATE_QUEUED))
> state = VB2_BUF_STATE_ERROR;
>
> @@ -1816,6 +1817,9 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
> case VB2_BUF_STATE_ERROR:
> dprintk(3, "returning done buffer with errors\n");
> break;
> + case VB2_BUF_STATE_ERROR_TOO_SMALL:
> + dprintk(3, "returning done buffer that's too small\n");
> + break;
> default:
> dprintk(1, "invalid buffer state\n");
> return -EINVAL;
> @@ -2383,8 +2387,9 @@ __poll_t vb2_core_poll(struct vb2_queue *q, struct file *file,
> done_entry);
> spin_unlock_irqrestore(&q->done_lock, flags);
>
> - if (vb && (vb->state == VB2_BUF_STATE_DONE
> - || vb->state == VB2_BUF_STATE_ERROR)) {
> + if (vb && (vb->state == VB2_BUF_STATE_DONE ||
> + vb->state == VB2_BUF_STATE_ERROR ||
> + vb->state == VB2_BUF_STATE_ERROR_TOO_SMALL)) {
> return (q->is_output) ?
> EPOLLOUT | EPOLLWRNORM :
> EPOLLIN | EPOLLRDNORM;
> @@ -2812,7 +2817,8 @@ static int vb2_thread(void *data)
> break;
> try_to_freeze();
>
> - if (vb->state != VB2_BUF_STATE_ERROR)
> + if (vb->state != VB2_BUF_STATE_ERROR &&
> + vb->state != VB2_BUF_STATE_ERROR_TOO_SMALL)
> if (threadio->fnc(vb, threadio->priv))
> break;
> call_void_qop(q, wait_finish, q);
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index e652f4318284..6ac19734e4a2 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -44,6 +44,7 @@ module_param(debug, int, 0644);
> /* Flags that are set by us */
> #define V4L2_BUFFER_MASK_FLAGS (V4L2_BUF_FLAG_MAPPED | V4L2_BUF_FLAG_QUEUED | \
> V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_ERROR | \
> + V4L2_BUF_FLAG_ERROR_TOO_SMALL | \
Oops, should have been V4L2_BUF_FLAG_TOO_SMALL. Sorry about that.
Regards,
Hans
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/5] Stateful Encoding: final bits
2019-11-19 11:34 [PATCH 0/5] Stateful Encoding: final bits Hans Verkuil
` (4 preceding siblings ...)
2019-11-19 11:34 ` [PATCH 5/5] media: docs-rst: Document memory-to-memory video encoder interface Hans Verkuil
@ 2019-11-19 16:15 ` Michael Tretter
2019-12-20 13:47 ` Michael Tretter
6 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2019-11-19 16:15 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media
Hi Hans,
On Tue, 19 Nov 2019 12:34:52 +0100, Hans Verkuil wrote:
> This series adds support for fractions in the control framework,
> and a way to obtain the min and max values of compound controls
> such as v4l2_fract.
>
> Next it adds the V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE control to
> set the framerate for the encoder.
>
> The next patch adds support for the V4L2_BUF_FLAG_TOO_SMALL flag
> to signal that the capture buffer was too small.
>
> The final patch adds the encoder spec (unchanged from v3).
>
> Michael, can you add support for V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE
> to your encoder driver? Let me know if something isn't working.
Thanks. Add will add the control and send patches.
> I need to add a test control for this to vivid as well and add support
> for this to v4l2-ctl, that's on my TODO list.
>
> Open questions:
>
> 1) Existing encoder drivers use S_PARM to signal the frameperiod,
> but as discussed in Lyon this should be the rate at which frames are
> submitted for encoding, which can be different from
> V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE. But given the current use-cases
> I was wondering if calling S_PARM should set the ENC_FRAME_RATE
> control as well, and you would need to explicitly set the control
> after S_PARM if the two are not the same. This would mean that
> existing applications that don't know about the control can keep working.
I am slightly confused, because I thought that S_PARM and
V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE should be used exactly the other way
around, but it makes sense to use S_PARM for configuring the frame rate
for producing frames in the hardware.
For encoding live the rate at which frames are submitted to the encoder
is the same as the framerate of the produced stream. They only differ
for use cases where we want to encode faster or slower than the
playback rate of the resulting stream. I guess assuming that they are
by default the same and only adapt the control if necessary should be
fine.
Michael
>
> 2) I do believe that we need a RELEASE/DEL/DESTROY_BUFS ioctl in
> order to do proper handling of the V4L2_BUF_FLAG_TOO_SMALL case.
> I am inclined to postpone adding this flag until we have that ioctl.
> We can still merge the stateful encoder spec if we mention that that
> is work in progress.
>
> Regards,
>
> Hans
>
> Hans Verkuil (4):
> v4l2-ctrls: add support for v4l2_fract types
> v4l2-ctrls: add support for V4L2_CTRL_WHICH_MIN/MAX_VAL
> v4l2-controls.h: add V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE
> videodev2.h: add V4L2_BUF_FLAG_TOO_SMALL flag
>
> Tomasz Figa (1):
> media: docs-rst: Document memory-to-memory video encoder interface
>
> Documentation/media/uapi/v4l/buffer.rst | 9 +
> Documentation/media/uapi/v4l/dev-encoder.rst | 608 ++++++++++++++++++
> Documentation/media/uapi/v4l/dev-mem2mem.rst | 1 +
> .../media/uapi/v4l/ext-ctrls-codec.rst | 8 +
> Documentation/media/uapi/v4l/pixfmt-v4l2.rst | 5 +
> Documentation/media/uapi/v4l/v4l2.rst | 2 +
> .../media/uapi/v4l/vidioc-encoder-cmd.rst | 51 +-
> .../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 15 +-
> .../media/uapi/v4l/vidioc-queryctrl.rst | 6 +
> .../media/videodev2.h.rst.exceptions | 3 +
> .../media/common/videobuf2/videobuf2-core.c | 12 +-
> .../media/common/videobuf2/videobuf2-v4l2.c | 4 +
> drivers/media/i2c/imx214.c | 4 +-
> drivers/media/v4l2-core/v4l2-ctrls.c | 222 ++++++-
> include/media/v4l2-ctrls.h | 72 ++-
> include/media/videobuf2-core.h | 4 +
> include/uapi/linux/v4l2-controls.h | 1 +
> include/uapi/linux/videodev2.h | 6 +
> 18 files changed, 980 insertions(+), 53 deletions(-)
> create mode 100644 Documentation/media/uapi/v4l/dev-encoder.rst
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/5] Stateful Encoding: final bits
2019-11-19 11:34 [PATCH 0/5] Stateful Encoding: final bits Hans Verkuil
` (5 preceding siblings ...)
2019-11-19 16:15 ` [PATCH 0/5] Stateful Encoding: final bits Michael Tretter
@ 2019-12-20 13:47 ` Michael Tretter
2019-12-20 13:48 ` [RFC PATCH] media: allegro: implement V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE Michael Tretter
` (2 more replies)
6 siblings, 3 replies; 13+ messages in thread
From: Michael Tretter @ 2019-12-20 13:47 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, kernel
Hello Hans,
On Tue, 19 Nov 2019 12:34:52 +0100, Hans Verkuil wrote:
> This series adds support for fractions in the control framework,
> and a way to obtain the min and max values of compound controls
> such as v4l2_fract.
>
> Next it adds the V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE control to
> set the framerate for the encoder.
>
> The next patch adds support for the V4L2_BUF_FLAG_TOO_SMALL flag
> to signal that the capture buffer was too small.
>
> The final patch adds the encoder spec (unchanged from v3).
>
> Michael, can you add support for V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE
> to your encoder driver? Let me know if something isn't working.
I implemented the control and hooked it up with S_PARM as well. The
implementation was straightforward without any real issues. I'll send a
patch in reply to this mail. Having a control for configuring the frame
rate that is encoded into the SPS feels correct. This is in line with
configuring the bitrate, level, etc.
However, after seeing the implementation and fiddling around with it in
userspace, I am not convinced that S_PARM should be used signal the
rate at which frames are submitted.
Setting the frame submission rate to something different than the
frame rate of the stream would be most interesting for transcoding use
cases. The user space would either want to run the encoding as fast as
possible or, if there are multiple encoding processes, as fast as
possible with properly shared resources. Boiling this information down
into a single number (and calling is "rate at which frames are
submitted") sounds kind of wrong, because the userspace does not know
which submission rate would lead to a good result.
Using the frame rate for such a setting seems pretty unique to the
allegro encoder. Other encoders might use different mechanisms to boost
the encoding speed, e.g., might be able to temporarily increase the
clock rate of the codec. In these cases, the driver would need to
translate the "framerate" set via S_PARM to a clock rate for the codec.
This does not sound right.
However, in the end, this would lead to exposing single parameters that
allow to tune the codec via generic controls. This does not seem to be
the right way, at all. Maybe we could have a control that tells the
encoder to "run as fast as possible" or to "run with as little
resources as possible", which would be applicable to more encoders and
the driver would have to decide how to implement this "profile".
Still, I am not really sure, if this is the proper way to solve this.
> I need to add a test control for this to vivid as well and add support
> for this to v4l2-ctl, that's on my TODO list.
>
> Open questions:
>
> 1) Existing encoder drivers use S_PARM to signal the frameperiod,
> but as discussed in Lyon this should be the rate at which frames are
> submitted for encoding, which can be different from
> V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE. But given the current use-cases
> I was wondering if calling S_PARM should set the ENC_FRAME_RATE
> control as well, and you would need to explicitly set the control
> after S_PARM if the two are not the same. This would mean that
> existing applications that don't know about the control can keep working.
In the patch I did exactly that and we should be backwards compatible
to applications that use only S_PARM.
Michael
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH] media: allegro: implement V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE
2019-12-20 13:47 ` Michael Tretter
@ 2019-12-20 13:48 ` Michael Tretter
2020-01-06 15:02 ` [PATCH 0/5] Stateful Encoding: final bits Hans Verkuil
2020-03-11 15:16 ` Nicolas Dufresne
2 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2019-12-20 13:48 UTC (permalink / raw)
To: hverkuil-cisco, linux-media; +Cc: kernel, Michael Tretter
Implement the new V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE control to set the
framerate in the encoded H.264 stream.
The current implementation conflicts with the specification. The
specification states that S_PARM sets the rate at which frames are
submitted to the encoder and V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE only
influences the frame rate that is put into the encoded stream. However,
the implementation uses the frame rate of the control in msg.framerate
for configuring the codec as well.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
.../staging/media/allegro-dvt/allegro-core.c | 72 +++++++++++++++++--
1 file changed, 67 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
index 6f0cd0784786..5631c56420cc 100644
--- a/drivers/staging/media/allegro-dvt/allegro-core.c
+++ b/drivers/staging/media/allegro-dvt/allegro-core.c
@@ -6,6 +6,7 @@
*/
#include <linux/firmware.h>
+#include <linux/gcd.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/kernel.h>
@@ -40,6 +41,8 @@
#define ALLEGRO_HEIGHT_DEFAULT 1080
#define ALLEGRO_HEIGHT_MAX 2160
+#define ALLEGRO_FRAMERATE_DEFAULT (struct v4l2_fract) { 30, 1 };
+
#define ALLEGRO_GOP_SIZE_DEFAULT 25
#define ALLEGRO_GOP_SIZE_MAX 1000
@@ -176,6 +179,7 @@ struct allegro_channel {
unsigned int width;
unsigned int height;
unsigned int stride;
+ struct v4l2_fract framerate;
enum v4l2_colorspace colorspace;
enum v4l2_ycbcr_encoding ycbcr_enc;
@@ -205,6 +209,7 @@ struct allegro_channel {
struct v4l2_ctrl *mpeg_video_bitrate_peak;
struct v4l2_ctrl *mpeg_video_cpb_size;
struct v4l2_ctrl *mpeg_video_gop_size;
+ struct v4l2_ctrl *mpeg_video_enc_frame_rate;
/* user_id is used to identify the channel during CREATE_CHANNEL */
/* not sure, what to set here and if this is actually required */
@@ -1048,8 +1053,9 @@ static int allegro_mcu_send_create_channel(struct allegro_dev *dev,
/* Encoder expects cpb_size in units of a 90 kHz clock. */
msg.cpb_size =
((channel->cpb_size * 1000) / channel->bitrate_peak) * 90000;
- msg.framerate = 25;
- msg.clk_ratio = 1000;
+ msg.framerate = DIV_ROUND_UP(channel->framerate.numerator,
+ channel->framerate.denominator);
+ msg.clk_ratio = channel->framerate.denominator == 1001 ? 1001 : 1000;
msg.target_bitrate = channel->bitrate;
msg.max_bitrate = channel->bitrate_peak;
msg.initial_qp = 25;
@@ -1360,9 +1366,11 @@ static ssize_t allegro_h264_write_sps(struct allegro_channel *channel,
sps->vui.chroma_loc_info_present_flag = 1;
sps->vui.chroma_sample_loc_type_top_field = 0;
sps->vui.chroma_sample_loc_type_bottom_field = 0;
+
sps->vui.timing_info_present_flag = 1;
- sps->vui.num_units_in_tick = 1;
- sps->vui.time_scale = 50;
+ sps->vui.num_units_in_tick = channel->framerate.denominator;
+ sps->vui.time_scale = 2 * channel->framerate.numerator;
+
sps->vui.fixed_frame_rate_flag = 1;
sps->vui.nal_hrd_parameters_present_flag = 0;
sps->vui.vcl_hrd_parameters_present_flag = 1;
@@ -2000,7 +2008,8 @@ static int allegro_create_channel(struct allegro_channel *channel)
v4l2_dbg(1, debug, &dev->v4l2_dev,
"user %d: creating channel (%4.4s, %dx%d@%d)\n",
channel->user_id,
- (char *)&channel->codec, channel->width, channel->height, 25);
+ (char *)&channel->codec, channel->width, channel->height,
+ DIV_ROUND_UP(channel->framerate.numerator, channel->framerate.denominator));
min_level = select_minimum_h264_level(channel->width, channel->height);
if (channel->level < min_level) {
@@ -2046,6 +2055,7 @@ static void allegro_set_default_params(struct allegro_channel *channel)
channel->width = ALLEGRO_WIDTH_DEFAULT;
channel->height = ALLEGRO_HEIGHT_DEFAULT;
channel->stride = round_up(channel->width, 32);
+ channel->framerate = ALLEGRO_FRAMERATE_DEFAULT;
channel->colorspace = V4L2_COLORSPACE_REC709;
channel->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
@@ -2231,6 +2241,7 @@ static int allegro_s_ctrl(struct v4l2_ctrl *ctrl)
struct allegro_channel,
ctrl_handler);
struct allegro_dev *dev = channel->dev;
+ int div;
v4l2_dbg(1, debug, &dev->v4l2_dev,
"s_ctrl: %s = %d\n", v4l2_ctrl_get_name(ctrl->id), ctrl->val);
@@ -2254,6 +2265,11 @@ static int allegro_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
channel->gop_size = ctrl->val;
break;
+ case V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE:
+ div = gcd(channel->framerate.numerator, channel->framerate.denominator);
+ channel->framerate.numerator = ctrl->p_new.p_fract->numerator / div;
+ channel->framerate.denominator = ctrl->p_new.p_fract->denominator / div;
+ break;
}
return 0;
@@ -2270,6 +2286,8 @@ static int allegro_open(struct file *file)
struct allegro_channel *channel = NULL;
struct v4l2_ctrl_handler *handler;
u64 mask;
+ struct v4l2_fract framerate_min = { 1, U32_MAX};
+ struct v4l2_fract framerate_max = { U32_MAX, 1};
channel = kzalloc(sizeof(*channel), GFP_KERNEL);
if (!channel)
@@ -2323,6 +2341,12 @@ static int allegro_open(struct file *file)
V4L2_CID_MPEG_VIDEO_GOP_SIZE,
0, ALLEGRO_GOP_SIZE_MAX,
1, channel->gop_size);
+ channel->mpeg_video_enc_frame_rate = v4l2_ctrl_new_std_compound(handler,
+ &allegro_ctrl_ops,
+ V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE,
+ v4l2_ctrl_ptr_create(&channel->framerate),
+ v4l2_ctrl_ptr_create(&framerate_min),
+ v4l2_ctrl_ptr_create(&framerate_max));
v4l2_ctrl_new_std(handler,
&allegro_ctrl_ops,
V4L2_CID_MIN_BUFFERS_FOR_OUTPUT,
@@ -2636,6 +2660,41 @@ static int allegro_ioctl_streamon(struct file *file, void *priv,
return v4l2_m2m_streamon(file, fh->m2m_ctx, type);
}
+static int allegro_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
+{
+ struct allegro_channel *channel = fh_to_channel(fh);
+ struct v4l2_fract *timeperframe;
+
+ if (a->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+ return -EINVAL;
+
+ a->parm.output.capability = V4L2_CAP_TIMEPERFRAME;
+ timeperframe = &a->parm.output.timeperframe;
+ timeperframe->numerator = channel->framerate.denominator;
+ timeperframe->denominator = channel->framerate.numerator;
+
+ return 0;
+}
+
+static int allegro_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
+{
+ struct allegro_channel *channel = fh_to_channel(fh);
+ struct v4l2_fract *timeperframe;
+ struct v4l2_fract framerate;
+
+ if (a->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+ return -EINVAL;
+
+ a->parm.output.capability = V4L2_CAP_TIMEPERFRAME;
+ timeperframe = &a->parm.output.timeperframe;
+ framerate.numerator = timeperframe->denominator;
+ framerate.denominator = timeperframe->numerator;
+
+ v4l2_ctrl_s_ctrl_fract(channel->mpeg_video_enc_frame_rate, &framerate);
+
+ return 0;
+}
+
static int allegro_subscribe_event(struct v4l2_fh *fh,
const struct v4l2_event_subscription *sub)
{
@@ -2674,6 +2733,9 @@ static const struct v4l2_ioctl_ops allegro_ioctl_ops = {
.vidioc_encoder_cmd = allegro_encoder_cmd,
.vidioc_enum_framesizes = allegro_enum_framesizes,
+ .vidioc_g_parm = allegro_g_parm,
+ .vidioc_s_parm = allegro_s_parm,
+
.vidioc_subscribe_event = allegro_subscribe_event,
.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
};
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 0/5] Stateful Encoding: final bits
2019-12-20 13:47 ` Michael Tretter
2019-12-20 13:48 ` [RFC PATCH] media: allegro: implement V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE Michael Tretter
@ 2020-01-06 15:02 ` Hans Verkuil
2020-01-18 13:14 ` Nicolas Dufresne
2020-03-11 15:16 ` Nicolas Dufresne
2 siblings, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2020-01-06 15:02 UTC (permalink / raw)
To: Michael Tretter; +Cc: linux-media, kernel, Nicolas Dufresne
(Added Nicolas as I'd like his input as well)
Reply below:
On 12/20/19 2:47 PM, Michael Tretter wrote:
> Hello Hans,
>
> On Tue, 19 Nov 2019 12:34:52 +0100, Hans Verkuil wrote:
>> This series adds support for fractions in the control framework,
>> and a way to obtain the min and max values of compound controls
>> such as v4l2_fract.
>>
>> Next it adds the V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE control to
>> set the framerate for the encoder.
>>
>> The next patch adds support for the V4L2_BUF_FLAG_TOO_SMALL flag
>> to signal that the capture buffer was too small.
>>
>> The final patch adds the encoder spec (unchanged from v3).
>>
>> Michael, can you add support for V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE
>> to your encoder driver? Let me know if something isn't working.
>
> I implemented the control and hooked it up with S_PARM as well. The
> implementation was straightforward without any real issues. I'll send a
> patch in reply to this mail. Having a control for configuring the frame
> rate that is encoded into the SPS feels correct. This is in line with
> configuring the bitrate, level, etc.
>
> However, after seeing the implementation and fiddling around with it in
> userspace, I am not convinced that S_PARM should be used signal the
> rate at which frames are submitted.
>
> Setting the frame submission rate to something different than the
> frame rate of the stream would be most interesting for transcoding use
> cases. The user space would either want to run the encoding as fast as
> possible or, if there are multiple encoding processes, as fast as
> possible with properly shared resources. Boiling this information down
> into a single number (and calling is "rate at which frames are
> submitted") sounds kind of wrong, because the userspace does not know
> which submission rate would lead to a good result.
>
> Using the frame rate for such a setting seems pretty unique to the
> allegro encoder. Other encoders might use different mechanisms to boost
> the encoding speed, e.g., might be able to temporarily increase the
> clock rate of the codec. In these cases, the driver would need to
> translate the "framerate" set via S_PARM to a clock rate for the codec.
> This does not sound right.
>
> However, in the end, this would lead to exposing single parameters that
> allow to tune the codec via generic controls. This does not seem to be
> the right way, at all. Maybe we could have a control that tells the
> encoder to "run as fast as possible" or to "run with as little
> resources as possible", which would be applicable to more encoders and
> the driver would have to decide how to implement this "profile".
>
> Still, I am not really sure, if this is the proper way to solve this.
I think you raise good points.
So this means that we need this new control (required for stateful encoders)
and optionally allow S_PARM to be used as an alternative way to set the
same control.
I prefer to make S_PARM optional and require the control, since I hate S_PARM :-)
It would mean that all existing stateful encoders need to implement this new
control, but I think that's a good idea anyway.
>
>> I need to add a test control for this to vivid as well and add support
>> for this to v4l2-ctl, that's on my TODO list.
>>
>> Open questions:
>>
>> 1) Existing encoder drivers use S_PARM to signal the frameperiod,
>> but as discussed in Lyon this should be the rate at which frames are
>> submitted for encoding, which can be different from
>> V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE. But given the current use-cases
>> I was wondering if calling S_PARM should set the ENC_FRAME_RATE
>> control as well, and you would need to explicitly set the control
>> after S_PARM if the two are not the same. This would mean that
>> existing applications that don't know about the control can keep working.
>
> In the patch I did exactly that and we should be backwards compatible
> to applications that use only S_PARM.
>
> Michael
>
Thanks for working on this!
Happy New Year,
Hans
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/5] Stateful Encoding: final bits
2020-01-06 15:02 ` [PATCH 0/5] Stateful Encoding: final bits Hans Verkuil
@ 2020-01-18 13:14 ` Nicolas Dufresne
0 siblings, 0 replies; 13+ messages in thread
From: Nicolas Dufresne @ 2020-01-18 13:14 UTC (permalink / raw)
To: Hans Verkuil, Michael Tretter; +Cc: linux-media, kernel
Le lundi 06 janvier 2020 à 16:02 +0100, Hans Verkuil a écrit :
> (Added Nicolas as I'd like his input as well)
Sorry for the delay, I was pretty far behind on tracking this ML.
>
> Reply below:
>
> On 12/20/19 2:47 PM, Michael Tretter wrote:
> > Hello Hans,
> >
> > On Tue, 19 Nov 2019 12:34:52 +0100, Hans Verkuil wrote:
> > > This series adds support for fractions in the control framework,
> > > and a way to obtain the min and max values of compound controls
> > > such as v4l2_fract.
> > >
> > > Next it adds the V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE control to
> > > set the framerate for the encoder.
> > >
> > > The next patch adds support for the V4L2_BUF_FLAG_TOO_SMALL flag
> > > to signal that the capture buffer was too small.
> > >
> > > The final patch adds the encoder spec (unchanged from v3).
> > >
> > > Michael, can you add support for V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE
> > > to your encoder driver? Let me know if something isn't working.
> >
> > I implemented the control and hooked it up with S_PARM as well. The
> > implementation was straightforward without any real issues. I'll send a
> > patch in reply to this mail. Having a control for configuring the frame
> > rate that is encoded into the SPS feels correct. This is in line with
> > configuring the bitrate, level, etc.
> >
> > However, after seeing the implementation and fiddling around with it in
> > userspace, I am not convinced that S_PARM should be used signal the
> > rate at which frames are submitted.
> >
> > Setting the frame submission rate to something different than the
> > frame rate of the stream would be most interesting for transcoding use
> > cases. The user space would either want to run the encoding as fast as
> > possible or, if there are multiple encoding processes, as fast as
> > possible with properly shared resources. Boiling this information down
> > into a single number (and calling is "rate at which frames are
> > submitted") sounds kind of wrong, because the userspace does not know
> > which submission rate would lead to a good result.
> >
> > Using the frame rate for such a setting seems pretty unique to the
> > allegro encoder. Other encoders might use different mechanisms to boost
> > the encoding speed, e.g., might be able to temporarily increase the
> > clock rate of the codec. In these cases, the driver would need to
> > translate the "framerate" set via S_PARM to a clock rate for the codec.
> > This does not sound right.
> >
> > However, in the end, this would lead to exposing single parameters that
> > allow to tune the codec via generic controls. This does not seem to be
> > the right way, at all. Maybe we could have a control that tells the
> > encoder to "run as fast as possible" or to "run with as little
> > resources as possible", which would be applicable to more encoders and
> > the driver would have to decide how to implement this "profile".
> >
> > Still, I am not really sure, if this is the proper way to solve this.
>
> I think you raise good points.
>
> So this means that we need this new control (required for stateful encoders)
> and optionally allow S_PARM to be used as an alternative way to set the
> same control.
Indeed that rase a strong point. In all scenarios I have in mind you're analyses
is correct. It's binary:
a) This is live and then the frame rate and the speed matches
b) This is offline processing, and then we'd like to use as much power as possible
What I think is important in what you raise, is that unlike the Allegro encoder,
other encoders may be able to control usage in a more dynamic ways which indeed
is not guarantied to correlate with framerate (could be utilisation driver clock
scaling). Then yes, a control to tell the Allegro encoder that we are doing
offline processing would work, though it can be optional, as other encoder with
more dynamic performance scaling won't need that hint to work.
>
> I prefer to make S_PARM optional and require the control, since I hate S_PARM
> :-)
>
> It would mean that all existing stateful encoders need to implement this new
> control, but I think that's a good idea anyway.
I share your preference, but that means more userspace backward compatibility
code is needed. Notably, I'll have try the new one and fallback for this case to
stay compatible with older kernel.
>
> > > I need to add a test control for this to vivid as well and add support
> > > for this to v4l2-ctl, that's on my TODO list.
> > >
> > > Open questions:
> > >
> > > 1) Existing encoder drivers use S_PARM to signal the frameperiod,
> > > but as discussed in Lyon this should be the rate at which frames are
> > > submitted for encoding, which can be different from
> > > V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE. But given the current use-cases
> > > I was wondering if calling S_PARM should set the ENC_FRAME_RATE
> > > control as well, and you would need to explicitly set the control
> > > after S_PARM if the two are not the same. This would mean that
> > > existing applications that don't know about the control can keep working.
> >
> > In the patch I did exactly that and we should be backwards compatible
> > to applications that use only S_PARM.
> >
> > Michael
> >
>
> Thanks for working on this!
>
> Happy New Year,
>
> Hans
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/5] Stateful Encoding: final bits
2019-12-20 13:47 ` Michael Tretter
2019-12-20 13:48 ` [RFC PATCH] media: allegro: implement V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE Michael Tretter
2020-01-06 15:02 ` [PATCH 0/5] Stateful Encoding: final bits Hans Verkuil
@ 2020-03-11 15:16 ` Nicolas Dufresne
2 siblings, 0 replies; 13+ messages in thread
From: Nicolas Dufresne @ 2020-03-11 15:16 UTC (permalink / raw)
To: Michael Tretter, Hans Verkuil; +Cc: linux-media, kernel
Le vendredi 20 décembre 2019 à 14:47 +0100, Michael Tretter a écrit :
> Hello Hans,
>
> On Tue, 19 Nov 2019 12:34:52 +0100, Hans Verkuil wrote:
> > This series adds support for fractions in the control framework,
> > and a way to obtain the min and max values of compound controls
> > such as v4l2_fract.
> >
> > Next it adds the V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE control to
> > set the framerate for the encoder.
> >
> > The next patch adds support for the V4L2_BUF_FLAG_TOO_SMALL flag
> > to signal that the capture buffer was too small.
> >
> > The final patch adds the encoder spec (unchanged from v3).
> >
> > Michael, can you add support for V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE
> > to your encoder driver? Let me know if something isn't working.
>
> I implemented the control and hooked it up with S_PARM as well. The
> implementation was straightforward without any real issues. I'll send a
> patch in reply to this mail. Having a control for configuring the frame
> rate that is encoded into the SPS feels correct. This is in line with
> configuring the bitrate, level, etc.
>
> However, after seeing the implementation and fiddling around with it in
> userspace, I am not convinced that S_PARM should be used signal the
> rate at which frames are submitted.
>
> Setting the frame submission rate to something different than the
> frame rate of the stream would be most interesting for transcoding use
> cases. The user space would either want to run the encoding as fast as
> possible or, if there are multiple encoding processes, as fast as
> possible with properly shared resources. Boiling this information down
> into a single number (and calling is "rate at which frames are
> submitted") sounds kind of wrong, because the userspace does not know
> which submission rate would lead to a good result.
>
> Using the frame rate for such a setting seems pretty unique to the
> allegro encoder. Other encoders might use different mechanisms to boost
> the encoding speed, e.g., might be able to temporarily increase the
> clock rate of the codec. In these cases, the driver would need to
> translate the "framerate" set via S_PARM to a clock rate for the codec.
> This does not sound right.
>
> However, in the end, this would lead to exposing single parameters that
> allow to tune the codec via generic controls. This does not seem to be
> the right way, at all. Maybe we could have a control that tells the
> encoder to "run as fast as possible" or to "run with as little
> resources as possible", which would be applicable to more encoders and
> the driver would have to decide how to implement this "profile".
>
> Still, I am not really sure, if this is the proper way to solve this.
>
> > I need to add a test control for this to vivid as well and add support
> > for this to v4l2-ctl, that's on my TODO list.
> >
> > Open questions:
> >
> > 1) Existing encoder drivers use S_PARM to signal the frameperiod,
> > but as discussed in Lyon this should be the rate at which frames are
> > submitted for encoding, which can be different from
> > V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE. But given the current use-cases
> > I was wondering if calling S_PARM should set the ENC_FRAME_RATE
> > control as well, and you would need to explicitly set the control
> > after S_PARM if the two are not the same. This would mean that
> > existing applications that don't know about the control can keep working.
>
> In the patch I did exactly that and we should be backwards compatible
> to applications that use only S_PARM.
As per today's IRC discussion, adding a new FRAME_RATE control will in the end
only move a functionality from one place to another with a different form. If we
want to be reasonnable, despite our common dislike of s_parm, I believe we
should stay were we are with S_PARM, and just make sure drivers don't use this
to scale the HW performance, or not make this the default at least.
In the end, the S_PARM will tell the encoder what to do for CBR with a B/s
configuration and will allow the driver to calculate the contraints to be
written into bitstream headers when supported by the bitstream.
What I'm suggesting for now is to scope out this change until we have a better
reason to ask userspace folks to port. Is that reasonnable ? And we can forcus
on other aspects.
>
> Michael
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-03-11 15:16 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-19 11:34 [PATCH 0/5] Stateful Encoding: final bits Hans Verkuil
2019-11-19 11:34 ` [PATCH 1/5] v4l2-ctrls: add support for v4l2_fract types Hans Verkuil
2019-11-19 11:34 ` [PATCH 2/5] v4l2-ctrls: add support for V4L2_CTRL_WHICH_MIN/MAX_VAL Hans Verkuil
2019-11-19 11:34 ` [PATCH 3/5] v4l2-controls.h: add V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE Hans Verkuil
2019-11-19 11:34 ` [PATCH 4/5] videodev2.h: add V4L2_BUF_FLAG_TOO_SMALL flag Hans Verkuil
2019-11-19 14:36 ` Hans Verkuil
2019-11-19 11:34 ` [PATCH 5/5] media: docs-rst: Document memory-to-memory video encoder interface Hans Verkuil
2019-11-19 16:15 ` [PATCH 0/5] Stateful Encoding: final bits Michael Tretter
2019-12-20 13:47 ` Michael Tretter
2019-12-20 13:48 ` [RFC PATCH] media: allegro: implement V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE Michael Tretter
2020-01-06 15:02 ` [PATCH 0/5] Stateful Encoding: final bits Hans Verkuil
2020-01-18 13:14 ` Nicolas Dufresne
2020-03-11 15:16 ` Nicolas Dufresne
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).