Re: [PATCH v2 1/2] v4l: Add resolution change event.
On 05/05/2014 11:50 AM, Arun Kumar K wrote:
> Hi Hans,
>
> On Wed, Apr 30, 2014 at 8:03 PM, Hans Verkuil wrote:
>> On 04/30/2014 12:38 PM, Arun Kumar K wrote:
>>> Hi Hans,
>>>
>>>
>>> On 04/22/14 18:22, Hans Verkuil wrote:
On 04/21/2014 11:26 AM, Arun Kumar K wrote:
> From: Pawel Osciak
>
> This event indicates that the decoder has reached a point in the stream,
> at which the resolution changes. The userspace is expected to provide a
> new
> set of CAPTURE buffers for the new format before decoding can continue.
> The event can also be used for more generic events involving resolution
> or format changes at runtime for all kinds of video devices.
>
> Signed-off-by: Pawel Osciak
> Signed-off-by: Arun Kumar K
> ---
> .../DocBook/media/v4l/vidioc-subscribe-event.xml | 16
>
> include/uapi/linux/videodev2.h |6 ++
> 2 files changed, 22 insertions(+)
>
> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> index 5c70b61..0aec831 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> @@ -155,6 +155,22 @@
>
>
>
> + V4L2_EVENT_SOURCE_CHANGE
> + 5
> +
> + This event is triggered when a resolution or format change
> + is detected during runtime by the video device. It can be a
> + runtime resolution change triggered by a video decoder or the
> + format change happening on an HDMI connector. Application may
> + need to reinitialize buffers before proceeding further.
> +
> + This event has a &v4l2-event-source-change;
> associated
> + with it. This has significance only for v4l2 subdevs where the
> + pad_num field will be updated with
> + the pad number on which the event is triggered.
> +
> +
> +
> V4L2_EVENT_PRIVATE_START
> 0x0800
> Base event number for driver-private events.
> diff --git a/include/uapi/linux/videodev2.h
> b/include/uapi/linux/videodev2.h
> index 6ae7bbe..12e0614 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1733,6 +1733,7 @@ struct v4l2_streamparm {
> #define V4L2_EVENT_EOS 2
> #define V4L2_EVENT_CTRL3
> #define V4L2_EVENT_FRAME_SYNC 4
> +#define V4L2_EVENT_SOURCE_CHANGE 5
> #define V4L2_EVENT_PRIVATE_START 0x0800
>
> /* Payload for V4L2_EVENT_VSYNC */
> @@ -1764,12 +1765,17 @@ struct v4l2_event_frame_sync {
> __u32 frame_sequence;
> };
>
> +struct v4l2_event_source_change {
> + __u32 pad_num;
> +};
> +
> struct v4l2_event {
> __u32 type;
> union {
> struct v4l2_event_vsync vsync;
> struct v4l2_event_ctrl ctrl;
> struct v4l2_event_frame_syncframe_sync;
> + struct v4l2_event_source_change source_change;
> __u8data[64];
> } u;
> __u32 pending;
>
This needs to be done differently. The problem is that you really have
multiple
events that you want to subscribe to: one for each pad (or input: see the
way
VIDIOC_G/S_EDID maps pad to an input or output index when used with a
video node,
we have to support that for this event as well).
What you want to do is similar to what is done for control events: there
you can
subscribe for a specific control and get notified when that control
changes.
The way that works in the event code is that the 'id' field in the
v4l2_event
struct contains that control ID, or, in this case, the pad/input/output
index.
>>>
>>> As I am new to v4l2-events itself, I might have some noob questions.
>>> I understood filling pad index into id field. But for video nodes,
>>> the application is supposed to put v4l2_buf_type in the id field?
>>
>> No, a capture video node can have multiple inputs (those are enumerated with
>> VIDIOC_ENUMINPUTS), and each input can generate a SOURCE_CHANGE event, even
>> if it
>> is not the currently active input. Hence the need of setting id to the input
>> index. v4l2_buf_type makes no sense here.
>>
>
> Ok. But in a simple m2m device like mfc, the userspace has to subscribe to
> the event say on the CAPTURE plane where buffers has to be re-allocated on
> resolution change. So in such a case, what should be put in id fiel
Re: [PATCH v2 1/2] v4l: Add resolution change event.
Hi Hans,
On Wed, Apr 30, 2014 at 8:03 PM, Hans Verkuil wrote:
> On 04/30/2014 12:38 PM, Arun Kumar K wrote:
>> Hi Hans,
>>
>>
>> On 04/22/14 18:22, Hans Verkuil wrote:
>>> On 04/21/2014 11:26 AM, Arun Kumar K wrote:
From: Pawel Osciak
This event indicates that the decoder has reached a point in the stream,
at which the resolution changes. The userspace is expected to provide a new
set of CAPTURE buffers for the new format before decoding can continue.
The event can also be used for more generic events involving resolution
or format changes at runtime for all kinds of video devices.
Signed-off-by: Pawel Osciak
Signed-off-by: Arun Kumar K
---
.../DocBook/media/v4l/vidioc-subscribe-event.xml | 16
include/uapi/linux/videodev2.h |6 ++
2 files changed, 22 insertions(+)
diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
index 5c70b61..0aec831 100644
--- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
@@ -155,6 +155,22 @@
+ V4L2_EVENT_SOURCE_CHANGE
+ 5
+
+ This event is triggered when a resolution or format change
+ is detected during runtime by the video device. It can be a
+ runtime resolution change triggered by a video decoder or the
+ format change happening on an HDMI connector. Application may
+ need to reinitialize buffers before proceeding further.
+
+ This event has a &v4l2-event-source-change; associated
+ with it. This has significance only for v4l2 subdevs where the
+ pad_num field will be updated with
+ the pad number on which the event is triggered.
+
+
+
V4L2_EVENT_PRIVATE_START
0x0800
Base event number for driver-private events.
diff --git a/include/uapi/linux/videodev2.h
b/include/uapi/linux/videodev2.h
index 6ae7bbe..12e0614 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1733,6 +1733,7 @@ struct v4l2_streamparm {
#define V4L2_EVENT_EOS 2
#define V4L2_EVENT_CTRL3
#define V4L2_EVENT_FRAME_SYNC 4
+#define V4L2_EVENT_SOURCE_CHANGE 5
#define V4L2_EVENT_PRIVATE_START 0x0800
/* Payload for V4L2_EVENT_VSYNC */
@@ -1764,12 +1765,17 @@ struct v4l2_event_frame_sync {
__u32 frame_sequence;
};
+struct v4l2_event_source_change {
+ __u32 pad_num;
+};
+
struct v4l2_event {
__u32 type;
union {
struct v4l2_event_vsync vsync;
struct v4l2_event_ctrl ctrl;
struct v4l2_event_frame_syncframe_sync;
+ struct v4l2_event_source_change source_change;
__u8data[64];
} u;
__u32 pending;
>>>
>>> This needs to be done differently. The problem is that you really have
>>> multiple
>>> events that you want to subscribe to: one for each pad (or input: see the
>>> way
>>> VIDIOC_G/S_EDID maps pad to an input or output index when used with a video
>>> node,
>>> we have to support that for this event as well).
>>>
>>> What you want to do is similar to what is done for control events: there
>>> you can
>>> subscribe for a specific control and get notified when that control changes.
>>>
>>> The way that works in the event code is that the 'id' field in the
>>> v4l2_event
>>> struct contains that control ID, or, in this case, the pad/input/output
>>> index.
>>>
>>
>> As I am new to v4l2-events itself, I might have some noob questions.
>> I understood filling pad index into id field. But for video nodes,
>> the application is supposed to put v4l2_buf_type in the id field?
>
> No, a capture video node can have multiple inputs (those are enumerated with
> VIDIOC_ENUMINPUTS), and each input can generate a SOURCE_CHANGE event, even
> if it
> is not the currently active input. Hence the need of setting id to the input
> index. v4l2_buf_type makes no sense here.
>
Ok. But in a simple m2m device like mfc, the userspace has to subscribe to
the event say on the CAPTURE plane where buffers has to be re-allocated on
resolution change. So in such a case, what should be put in id field?
>>
>>> In the application you will subscribe to the SOURCE_CHANGE event for a
>>> specific
>>> pad/input/output index.
>>>
>>> In other words, the pad_num field should be dropped and the id
Re: [PATCH v2 1/2] v4l: Add resolution change event.
On 04/30/2014 12:38 PM, Arun Kumar K wrote:
> Hi Hans,
>
>
> On 04/22/14 18:22, Hans Verkuil wrote:
>> On 04/21/2014 11:26 AM, Arun Kumar K wrote:
>>> From: Pawel Osciak
>>>
>>> This event indicates that the decoder has reached a point in the stream,
>>> at which the resolution changes. The userspace is expected to provide a new
>>> set of CAPTURE buffers for the new format before decoding can continue.
>>> The event can also be used for more generic events involving resolution
>>> or format changes at runtime for all kinds of video devices.
>>>
>>> Signed-off-by: Pawel Osciak
>>> Signed-off-by: Arun Kumar K
>>> ---
>>> .../DocBook/media/v4l/vidioc-subscribe-event.xml | 16
>>> include/uapi/linux/videodev2.h |6 ++
>>> 2 files changed, 22 insertions(+)
>>>
>>> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>>> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>>> index 5c70b61..0aec831 100644
>>> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>>> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>>> @@ -155,6 +155,22 @@
>>>
>>>
>>>
>>> + V4L2_EVENT_SOURCE_CHANGE
>>> + 5
>>> +
>>> + This event is triggered when a resolution or format change
>>> + is detected during runtime by the video device. It can be a
>>> + runtime resolution change triggered by a video decoder or the
>>> + format change happening on an HDMI connector. Application may
>>> + need to reinitialize buffers before proceeding further.
>>> +
>>> + This event has a &v4l2-event-source-change; associated
>>> + with it. This has significance only for v4l2 subdevs where the
>>> + pad_num field will be updated with
>>> + the pad number on which the event is triggered.
>>> +
>>> +
>>> +
>>> V4L2_EVENT_PRIVATE_START
>>> 0x0800
>>> Base event number for driver-private events.
>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>> index 6ae7bbe..12e0614 100644
>>> --- a/include/uapi/linux/videodev2.h
>>> +++ b/include/uapi/linux/videodev2.h
>>> @@ -1733,6 +1733,7 @@ struct v4l2_streamparm {
>>> #define V4L2_EVENT_EOS 2
>>> #define V4L2_EVENT_CTRL3
>>> #define V4L2_EVENT_FRAME_SYNC 4
>>> +#define V4L2_EVENT_SOURCE_CHANGE 5
>>> #define V4L2_EVENT_PRIVATE_START 0x0800
>>>
>>> /* Payload for V4L2_EVENT_VSYNC */
>>> @@ -1764,12 +1765,17 @@ struct v4l2_event_frame_sync {
>>> __u32 frame_sequence;
>>> };
>>>
>>> +struct v4l2_event_source_change {
>>> + __u32 pad_num;
>>> +};
>>> +
>>> struct v4l2_event {
>>> __u32 type;
>>> union {
>>> struct v4l2_event_vsync vsync;
>>> struct v4l2_event_ctrl ctrl;
>>> struct v4l2_event_frame_syncframe_sync;
>>> + struct v4l2_event_source_change source_change;
>>> __u8data[64];
>>> } u;
>>> __u32 pending;
>>>
>>
>> This needs to be done differently. The problem is that you really have
>> multiple
>> events that you want to subscribe to: one for each pad (or input: see the way
>> VIDIOC_G/S_EDID maps pad to an input or output index when used with a video
>> node,
>> we have to support that for this event as well).
>>
>> What you want to do is similar to what is done for control events: there you
>> can
>> subscribe for a specific control and get notified when that control changes.
>>
>> The way that works in the event code is that the 'id' field in the v4l2_event
>> struct contains that control ID, or, in this case, the pad/input/output
>> index.
>>
>
> As I am new to v4l2-events itself, I might have some noob questions.
> I understood filling pad index into id field. But for video nodes,
> the application is supposed to put v4l2_buf_type in the id field?
No, a capture video node can have multiple inputs (those are enumerated with
VIDIOC_ENUMINPUTS), and each input can generate a SOURCE_CHANGE event, even if
it
is not the currently active input. Hence the need of setting id to the input
index. v4l2_buf_type makes no sense here.
>
>> In the application you will subscribe to the SOURCE_CHANGE event for a
>> specific
>> pad/input/output index.
>>
>> In other words, the pad_num field should be dropped and the id field used
>> instead.
>>
>> Assuming we will also add a 'changes' field (see my reply to the other post
>> on that topic), then you should also provide replace and merge ops (see
>> v4l2-ctrls.c). That way it is sufficient to use just one element when calling
>> v4l2_event_subscribe(): you will never loose information since if multiple
>> events arrive before the application can dequeue them, the 'changes'
>
Re: [PATCH v2 1/2] v4l: Add resolution change event.
Hi Hans,
On 04/22/14 18:22, Hans Verkuil wrote:
> On 04/21/2014 11:26 AM, Arun Kumar K wrote:
>> From: Pawel Osciak
>>
>> This event indicates that the decoder has reached a point in the stream,
>> at which the resolution changes. The userspace is expected to provide a new
>> set of CAPTURE buffers for the new format before decoding can continue.
>> The event can also be used for more generic events involving resolution
>> or format changes at runtime for all kinds of video devices.
>>
>> Signed-off-by: Pawel Osciak
>> Signed-off-by: Arun Kumar K
>> ---
>> .../DocBook/media/v4l/vidioc-subscribe-event.xml | 16
>> include/uapi/linux/videodev2.h |6 ++
>> 2 files changed, 22 insertions(+)
>>
>> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> index 5c70b61..0aec831 100644
>> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> @@ -155,6 +155,22 @@
>>
>>
>>
>> +V4L2_EVENT_SOURCE_CHANGE
>> +5
>> +
>> + This event is triggered when a resolution or format change
>> + is detected during runtime by the video device. It can be a
>> + runtime resolution change triggered by a video decoder or the
>> + format change happening on an HDMI connector. Application may
>> + need to reinitialize buffers before proceeding further.
>> +
>> + This event has a &v4l2-event-source-change; associated
>> + with it. This has significance only for v4l2 subdevs where the
>> + pad_num field will be updated with
>> + the pad number on which the event is triggered.
>> +
>> +
>> +
>> V4L2_EVENT_PRIVATE_START
>> 0x0800
>> Base event number for driver-private events.
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index 6ae7bbe..12e0614 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -1733,6 +1733,7 @@ struct v4l2_streamparm {
>> #define V4L2_EVENT_EOS 2
>> #define V4L2_EVENT_CTRL 3
>> #define V4L2_EVENT_FRAME_SYNC 4
>> +#define V4L2_EVENT_SOURCE_CHANGE5
>> #define V4L2_EVENT_PRIVATE_START0x0800
>>
>> /* Payload for V4L2_EVENT_VSYNC */
>> @@ -1764,12 +1765,17 @@ struct v4l2_event_frame_sync {
>> __u32 frame_sequence;
>> };
>>
>> +struct v4l2_event_source_change {
>> +__u32 pad_num;
>> +};
>> +
>> struct v4l2_event {
>> __u32 type;
>> union {
>> struct v4l2_event_vsync vsync;
>> struct v4l2_event_ctrl ctrl;
>> struct v4l2_event_frame_syncframe_sync;
>> +struct v4l2_event_source_change source_change;
>> __u8data[64];
>> } u;
>> __u32 pending;
>>
>
> This needs to be done differently. The problem is that you really have
> multiple
> events that you want to subscribe to: one for each pad (or input: see the way
> VIDIOC_G/S_EDID maps pad to an input or output index when used with a video
> node,
> we have to support that for this event as well).
>
> What you want to do is similar to what is done for control events: there you
> can
> subscribe for a specific control and get notified when that control changes.
>
> The way that works in the event code is that the 'id' field in the v4l2_event
> struct contains that control ID, or, in this case, the pad/input/output index.
>
As I am new to v4l2-events itself, I might have some noob questions.
I understood filling pad index into id field. But for video nodes,
the application is supposed to put v4l2_buf_type in the id field?
> In the application you will subscribe to the SOURCE_CHANGE event for a
> specific
> pad/input/output index.
>
> In other words, the pad_num field should be dropped and the id field used
> instead.
>
> Assuming we will also add a 'changes' field (see my reply to the other post
> on that topic), then you should also provide replace and merge ops (see
> v4l2-ctrls.c). That way it is sufficient to use just one element when calling
> v4l2_event_subscribe(): you will never loose information since if multiple
> events arrive before the application can dequeue them, the 'changes'
> information
> will be the ORed value of all those intermediate events.
>
If I understand correctly, the implementation should go somewhat in the
following lines:
+void v4l2_event_src_replace(struct v4l2_event *old, const struct
v4l2_event *new)
+{
+ u32 old_changes = old->u.src_change.changes;
+
+ old->u.src_change = new->u.src_change;
+ old->u.src_change.changes |= old_changes;
+}
Re: [PATCH v2 1/2] v4l: Add resolution change event.
On 04/21/2014 11:26 AM, Arun Kumar K wrote:
> From: Pawel Osciak
>
> This event indicates that the decoder has reached a point in the stream,
> at which the resolution changes. The userspace is expected to provide a new
> set of CAPTURE buffers for the new format before decoding can continue.
> The event can also be used for more generic events involving resolution
> or format changes at runtime for all kinds of video devices.
>
> Signed-off-by: Pawel Osciak
> Signed-off-by: Arun Kumar K
> ---
> .../DocBook/media/v4l/vidioc-subscribe-event.xml | 16
> include/uapi/linux/videodev2.h |6 ++
> 2 files changed, 22 insertions(+)
>
> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> index 5c70b61..0aec831 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> @@ -155,6 +155,22 @@
>
>
>
> + V4L2_EVENT_SOURCE_CHANGE
> + 5
> +
> + This event is triggered when a resolution or format change
> +is detected during runtime by the video device. It can be a
> +runtime resolution change triggered by a video decoder or the
> +format change happening on an HDMI connector. Application may
> +need to reinitialize buffers before proceeding further.
> +
> + This event has a &v4l2-event-source-change; associated
> + with it. This has significance only for v4l2 subdevs where the
> + pad_num field will be updated with
> + the pad number on which the event is triggered.
> +
> +
> +
> V4L2_EVENT_PRIVATE_START
> 0x0800
> Base event number for driver-private events.
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 6ae7bbe..12e0614 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1733,6 +1733,7 @@ struct v4l2_streamparm {
> #define V4L2_EVENT_EOS 2
> #define V4L2_EVENT_CTRL 3
> #define V4L2_EVENT_FRAME_SYNC4
> +#define V4L2_EVENT_SOURCE_CHANGE 5
> #define V4L2_EVENT_PRIVATE_START 0x0800
>
> /* Payload for V4L2_EVENT_VSYNC */
> @@ -1764,12 +1765,17 @@ struct v4l2_event_frame_sync {
> __u32 frame_sequence;
> };
>
> +struct v4l2_event_source_change {
> + __u32 pad_num;
> +};
> +
> struct v4l2_event {
> __u32 type;
> union {
> struct v4l2_event_vsync vsync;
> struct v4l2_event_ctrl ctrl;
> struct v4l2_event_frame_syncframe_sync;
> + struct v4l2_event_source_change source_change;
> __u8data[64];
> } u;
> __u32 pending;
>
This needs to be done differently. The problem is that you really have multiple
events that you want to subscribe to: one for each pad (or input: see the way
VIDIOC_G/S_EDID maps pad to an input or output index when used with a video
node,
we have to support that for this event as well).
What you want to do is similar to what is done for control events: there you can
subscribe for a specific control and get notified when that control changes.
The way that works in the event code is that the 'id' field in the v4l2_event
struct contains that control ID, or, in this case, the pad/input/output index.
In the application you will subscribe to the SOURCE_CHANGE event for a specific
pad/input/output index.
In other words, the pad_num field should be dropped and the id field used
instead.
Assuming we will also add a 'changes' field (see my reply to the other post
on that topic), then you should also provide replace and merge ops (see
v4l2-ctrls.c). That way it is sufficient to use just one element when calling
v4l2_event_subscribe(): you will never loose information since if multiple
events arrive before the application can dequeue them, the 'changes' information
will be the ORed value of all those intermediate events.
It's all more work, but it is critical to ensure that the application never
misses events.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] v4l: Add resolution change event.
On 04/21/2014 10:59 PM, Laurent Pinchart wrote:
> Hi Arun,
>
> On Monday 21 April 2014 17:19:26 Arun Kumar K wrote:
>> On Mon, Apr 21, 2014 at 3:54 PM, Laurent Pinchart wrote:
>>> On Monday 21 April 2014 14:56:01 Arun Kumar K wrote:
From: Pawel Osciak
This event indicates that the decoder has reached a point in the stream,
at which the resolution changes. The userspace is expected to provide a
new
set of CAPTURE buffers for the new format before decoding can continue.
The event can also be used for more generic events involving resolution
or format changes at runtime for all kinds of video devices.
Signed-off-by: Pawel Osciak
Signed-off-by: Arun Kumar K
---
.../DocBook/media/v4l/vidioc-subscribe-event.xml | 16
include/uapi/linux/videodev2.h |6 ++
2 files changed, 22 insertions(+)
diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index
5c70b61..0aec831 100644
--- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
@@ -155,6 +155,22 @@
+ V4L2_EVENT_SOURCE_CHANGE
+ 5
+
+ This event is triggered when a resolution or format
change +is detected during runtime by the video device. It
can be a +runtime resolution change triggered by a video
decoder or the +format change happening on an HDMI
connector. Application may +need to reinitialize buffers
before proceeding further. +
+ This event has a &v4l2-event-source-change;
associated
+ with it. This has significance only for v4l2 subdevs where
the
+ pad_num field will be updated with
+ the pad number on which the event is triggered.
+
+
+
V4L2_EVENT_PRIVATE_START
0x0800
Base event number for driver-private events.
diff --git a/include/uapi/linux/videodev2.h
b/include/uapi/linux/videodev2.h index 6ae7bbe..12e0614 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1733,6 +1733,7 @@ struct v4l2_streamparm {
#define V4L2_EVENT_EOS 2
#define V4L2_EVENT_CTRL 3
#define V4L2_EVENT_FRAME_SYNC4
+#define V4L2_EVENT_SOURCE_CHANGE 5
#define V4L2_EVENT_PRIVATE_START 0x0800
/* Payload for V4L2_EVENT_VSYNC */
@@ -1764,12 +1765,17 @@ struct v4l2_event_frame_sync {
__u32 frame_sequence;
};
+struct v4l2_event_source_change {
+ __u32 pad_num;
>>>
>>> I would call the field just "pad",
>>
>> Ok.
>>
+};
+
struct v4l2_event {
__u32 type;
union {
struct v4l2_event_vsync vsync;
struct v4l2_event_ctrl ctrl;
struct v4l2_event_frame_syncframe_sync;
+ struct v4l2_event_source_change source_change;
__u8data[64];
>>>
>>> This looks pretty good to me, but I'm a bit concerned about future
>>> compatibility. We might need to report more information to userspace, and
>>> in particular what has been changed at the source (resolution, format,
>>> ...). In order to do so, we'll need to add a flag field to
>>> v4l2_event_source_change.
>>
>> Ok a flag can be added with bitfields for reporting specific event type.
>
> I don't think we need to add it now. Just making sure it can be added later
> without breaking the userspace API would be enough for me.
I would add it, I think it makes sense. It would be similar to the 'changes'
field in struct v4l2_event_ctrl.
>
>>> The next __u32 right after the source_change field must thus be zeroed. I
>>> see two ways of doing so:
>>>
>>> - zeroing the whole data array before setting event-specific data
>>> - adding a reserved must-be-zeroed field to v4l2_event_source_change
>>>
>>> I like the former better as it's more generic, but we then need to ensure
>>> that all drivers zero the whole data field correctly. Adding a new
>>> v4l2_event_init() function would help with that.
>>
>> Is that a good idea to have an init() function just for zeroing the data
>> field? If this is agreed upon, I can add this, but it can be easily missed
>> out by drivers.
>
> I'm not sure. On one hand it would be easier to catch missing calls if we use
> a dedicated function, as the function could set an initialized flag in the
> event structure that would be checked by
RE: [PATCH v2 1/2] v4l: Add resolution change event.
Hi Pawel, > From: Pawel Osciak [mailto:posc...@chromium.org] > Sent: Tuesday, April 22, 2014 9:46 AM > To: Kamil Debski > Cc: Arun Kumar K; linux-media@vger.kernel.org; linux-samsung-soc; > Sylwester Nawrocki; Hans Verkuil; Laurent Pinchart > Subject: Re: [PATCH v2 1/2] v4l: Add resolution change event. > > Hi Kamil, > > On Tue, Apr 22, 2014 at 4:34 PM, Kamil Debski > wrote: > > > Hi Pawel, > > > From: Pawel Osciak [mailto:posc...@chromium.org] > > Sent: Monday, April 21, 2014 12:27 PM > > To: Arun Kumar K > > Cc: linux-media@vger.kernel.org; linux-samsung-soc; Kamil > Debski; > > Sylwester Nawrocki; Hans Verkuil; Laurent Pinchart > > Subject: Re: [PATCH v2 1/2] v4l: Add resolution change event. > > > > > As a side note, because this is not really codified in the API, > I would > > like this event to indicate not only resolution change mid- > stream, but > > also detection of initial resolution, which should be a subset > of > > resolution change. I think this would make sense for the codec > > interface: > > > > Video decode: > > 1. S_FMT to given codec on OUTPUT queue. > > 2. REQBUFS(n) and STREAMON on OUTPUT queue. > > 3. Keep QBUFing until we get an resolution change event on the > CAPTURE > > queue; until then, the driver/codec HW will operate on the > OUTPUT queue > > only and try to detect relevant headers in the OUTPUT buffers, > and will > > send resolution change event once it finds resolution, profile, > etc. > > info). DQEVENT. > > 4. G_FMT on CAPTURE to get the discovered output format > (resolution), > > REQBUFS and STREAMON on the CAPTURE queue. > > 5. Normal mem-to-mem decoding. > > 6. If a resolution change event arrives on CAPTURE queue, > DQEVENT, > > STREAMOFF, REQBUFS(0) only on CAPTURE queue, and goto 4. OUTPUT > queue > > operates completely independently of this. > > > > Also, this event should invariably indicate all of the below: > > - all output buffers from before resolution change are already > ready on > > the CAPTURE queue to DQBUF (so it's ready to REQBUFS(0) after > DQBUFs), > > and > > - there will be no more new ready buffers on the CAPTURE queue > until > > the streamoff-reqbufs(0)-g_fmt-reqbufs()-streamon is performed, > and > > - OUTPUT queue is completely independent of all of the above > and can be > > still used as normal, i.e. stream buffers can still keep being > queued > > at any stage of the resolution change and they will be decoded > after > > resolution change sequence is finished; > > > > If we all agree to the above, I will prepare a subsequent patch > for the > > documentation to include the above. > > > If I understand correctly this will keep the old application > working. > By this I mean application that do not use events and rely on the > current > mechanism to detect initial header parsing and resolution change. > > If backward compatibility is kept I am all for the changes > proposed by you. > > > > MFC codec depends on the userspace to parse the stream and pass the > stream header with resolution info before calling G_FMT. So if it > ignores the events but keeps doing this, things should keep working I > think, as the G_FMT should still work as before. > > This event will help userspace that doesn't want to bother itself with > parsing the stream to know if the right header was queued and rely on > events to know when to call G_FMT instead. > > What do you think? Am I missing something? Do you think this will work with all MFC versions? I guess that your focus is on the newer MFC version such as v6 or even v7. I had worked mostly with the v5 version and I am not sure how well it will handle the situation when it does not receive the header as the first frame. Do you have a target with v5 of MFC at hand? I really like your idea. If you haven't got the possibility to check how it works on v5 then I suggest you go ahead with patches and later I will do some testing on MFC v5. Best wishes, -- Kamil Debski Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 1/2] v4l: Add resolution change event.
Hi Pawel, > From: Pawel Osciak [mailto:posc...@chromium.org] > Sent: Monday, April 21, 2014 12:27 PM > To: Arun Kumar K > Cc: linux-media@vger.kernel.org; linux-samsung-soc; Kamil Debski; > Sylwester Nawrocki; Hans Verkuil; Laurent Pinchart > Subject: Re: [PATCH v2 1/2] v4l: Add resolution change event. > > As a side note, because this is not really codified in the API, I would > like this event to indicate not only resolution change mid-stream, but > also detection of initial resolution, which should be a subset of > resolution change. I think this would make sense for the codec > interface: > > Video decode: > 1. S_FMT to given codec on OUTPUT queue. > 2. REQBUFS(n) and STREAMON on OUTPUT queue. > 3. Keep QBUFing until we get an resolution change event on the CAPTURE > queue; until then, the driver/codec HW will operate on the OUTPUT queue > only and try to detect relevant headers in the OUTPUT buffers, and will > send resolution change event once it finds resolution, profile, etc. > info). DQEVENT. > 4. G_FMT on CAPTURE to get the discovered output format (resolution), > REQBUFS and STREAMON on the CAPTURE queue. > 5. Normal mem-to-mem decoding. > 6. If a resolution change event arrives on CAPTURE queue, DQEVENT, > STREAMOFF, REQBUFS(0) only on CAPTURE queue, and goto 4. OUTPUT queue > operates completely independently of this. > > Also, this event should invariably indicate all of the below: > - all output buffers from before resolution change are already ready on > the CAPTURE queue to DQBUF (so it's ready to REQBUFS(0) after DQBUFs), > and > - there will be no more new ready buffers on the CAPTURE queue until > the streamoff-reqbufs(0)-g_fmt-reqbufs()-streamon is performed, and > - OUTPUT queue is completely independent of all of the above and can be > still used as normal, i.e. stream buffers can still keep being queued > at any stage of the resolution change and they will be decoded after > resolution change sequence is finished; > > If we all agree to the above, I will prepare a subsequent patch for the > documentation to include the above. If I understand correctly this will keep the old application working. By this I mean application that do not use events and rely on the current mechanism to detect initial header parsing and resolution change. If backward compatibility is kept I am all for the changes proposed by you. > > Thanks, > Pawel Best wishes, -- Kamil Debski Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] v4l: Add resolution change event.
Hi Arun,
On Monday 21 April 2014 17:19:26 Arun Kumar K wrote:
> On Mon, Apr 21, 2014 at 3:54 PM, Laurent Pinchart wrote:
> > On Monday 21 April 2014 14:56:01 Arun Kumar K wrote:
> >> From: Pawel Osciak
> >>
> >> This event indicates that the decoder has reached a point in the stream,
> >> at which the resolution changes. The userspace is expected to provide a
> >> new
> >> set of CAPTURE buffers for the new format before decoding can continue.
> >> The event can also be used for more generic events involving resolution
> >> or format changes at runtime for all kinds of video devices.
> >>
> >> Signed-off-by: Pawel Osciak
> >> Signed-off-by: Arun Kumar K
> >> ---
> >>
> >> .../DocBook/media/v4l/vidioc-subscribe-event.xml | 16
> >>
> >> include/uapi/linux/videodev2.h |6 ++
> >> 2 files changed, 22 insertions(+)
> >>
> >> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> >> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index
> >> 5c70b61..0aec831 100644
> >> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> >> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> >> @@ -155,6 +155,22 @@
> >>
> >>
> >>
> >> + V4L2_EVENT_SOURCE_CHANGE
> >> + 5
> >> +
> >> + This event is triggered when a resolution or format
> >> change +is detected during runtime by the video device. It
> >> can be a +runtime resolution change triggered by a video
> >> decoder or the +format change happening on an HDMI
> >> connector. Application may +need to reinitialize buffers
> >> before proceeding further. +
> >> + This event has a &v4l2-event-source-change;
> >> associated
> >> + with it. This has significance only for v4l2 subdevs where
> >> the
> >> + pad_num field will be updated with
> >> + the pad number on which the event is triggered.
> >> +
> >> +
> >> +
> >>
> >> V4L2_EVENT_PRIVATE_START
> >> 0x0800
> >> Base event number for driver-private events.
> >> diff --git a/include/uapi/linux/videodev2.h
> >> b/include/uapi/linux/videodev2.h index 6ae7bbe..12e0614 100644
> >> --- a/include/uapi/linux/videodev2.h
> >> +++ b/include/uapi/linux/videodev2.h
> >> @@ -1733,6 +1733,7 @@ struct v4l2_streamparm {
> >> #define V4L2_EVENT_EOS 2
> >> #define V4L2_EVENT_CTRL 3
> >> #define V4L2_EVENT_FRAME_SYNC4
> >> +#define V4L2_EVENT_SOURCE_CHANGE 5
> >> #define V4L2_EVENT_PRIVATE_START 0x0800
> >>
> >> /* Payload for V4L2_EVENT_VSYNC */
> >> @@ -1764,12 +1765,17 @@ struct v4l2_event_frame_sync {
> >> __u32 frame_sequence;
> >> };
> >>
> >> +struct v4l2_event_source_change {
> >> + __u32 pad_num;
> >
> > I would call the field just "pad",
>
> Ok.
>
> >> +};
> >> +
> >>
> >> struct v4l2_event {
> >> __u32 type;
> >> union {
> >> struct v4l2_event_vsync vsync;
> >> struct v4l2_event_ctrl ctrl;
> >> struct v4l2_event_frame_syncframe_sync;
> >> + struct v4l2_event_source_change source_change;
> >> __u8data[64];
> >
> > This looks pretty good to me, but I'm a bit concerned about future
> > compatibility. We might need to report more information to userspace, and
> > in particular what has been changed at the source (resolution, format,
> > ...). In order to do so, we'll need to add a flag field to
> > v4l2_event_source_change.
>
> Ok a flag can be added with bitfields for reporting specific event type.
I don't think we need to add it now. Just making sure it can be added later
without breaking the userspace API would be enough for me.
> > The next __u32 right after the source_change field must thus be zeroed. I
> > see two ways of doing so:
> >
> > - zeroing the whole data array before setting event-specific data
> > - adding a reserved must-be-zeroed field to v4l2_event_source_change
> >
> > I like the former better as it's more generic, but we then need to ensure
> > that all drivers zero the whole data field correctly. Adding a new
> > v4l2_event_init() function would help with that.
>
> Is that a good idea to have an init() function just for zeroing the data
> field? If this is agreed upon, I can add this, but it can be easily missed
> out by drivers.
I'm not sure. On one hand it would be easier to catch missing calls if we use
a dedicated function, as the function could set an initialized flag in the
event structure that would be checked by v4l2_event_queue. On the other hand,
that might be overengineering, and we could just manually check that all
drivers memset the structure to 0 before initializing fields. In b
Re: [PATCH v2 1/2] v4l: Add resolution change event.
Hi Laurent,
Thank you for the review.
On Mon, Apr 21, 2014 at 3:54 PM, Laurent Pinchart
wrote:
> Hi Arun,
>
> Thank you for the patch.
>
> On Monday 21 April 2014 14:56:01 Arun Kumar K wrote:
>> From: Pawel Osciak
>>
>> This event indicates that the decoder has reached a point in the stream,
>> at which the resolution changes. The userspace is expected to provide a new
>> set of CAPTURE buffers for the new format before decoding can continue.
>> The event can also be used for more generic events involving resolution
>> or format changes at runtime for all kinds of video devices.
>>
>> Signed-off-by: Pawel Osciak
>> Signed-off-by: Arun Kumar K
>> ---
>> .../DocBook/media/v4l/vidioc-subscribe-event.xml | 16
>> include/uapi/linux/videodev2.h |6 ++
>> 2 files changed, 22 insertions(+)
>>
>> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index
>> 5c70b61..0aec831 100644
>> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> @@ -155,6 +155,22 @@
>>
>>
>>
>> + V4L2_EVENT_SOURCE_CHANGE
>> + 5
>> +
>> + This event is triggered when a resolution or format change
>> +is detected during runtime by the video device. It can be a
>> +runtime resolution change triggered by a video decoder or the
>> +format change happening on an HDMI connector. Application may
>> +need to reinitialize buffers before proceeding further.
>> +
>> + This event has a &v4l2-event-source-change; associated
>> + with it. This has significance only for v4l2 subdevs where the
>> + pad_num field will be updated with
>> + the pad number on which the event is triggered.
>> +
>> +
>> +
>> V4L2_EVENT_PRIVATE_START
>> 0x0800
>> Base event number for driver-private events.
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index 6ae7bbe..12e0614 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -1733,6 +1733,7 @@ struct v4l2_streamparm {
>> #define V4L2_EVENT_EOS 2
>> #define V4L2_EVENT_CTRL 3
>> #define V4L2_EVENT_FRAME_SYNC4
>> +#define V4L2_EVENT_SOURCE_CHANGE 5
>> #define V4L2_EVENT_PRIVATE_START 0x0800
>>
>> /* Payload for V4L2_EVENT_VSYNC */
>> @@ -1764,12 +1765,17 @@ struct v4l2_event_frame_sync {
>> __u32 frame_sequence;
>> };
>>
>> +struct v4l2_event_source_change {
>> + __u32 pad_num;
>
> I would call the field just "pad",
>
Ok.
>> +};
>> +
>> struct v4l2_event {
>> __u32 type;
>> union {
>> struct v4l2_event_vsync vsync;
>> struct v4l2_event_ctrl ctrl;
>> struct v4l2_event_frame_syncframe_sync;
>> + struct v4l2_event_source_change source_change;
>> __u8data[64];
>
> This looks pretty good to me, but I'm a bit concerned about future
> compatibility. We might need to report more information to userspace, and in
> particular what has been changed at the source (resolution, format, ...). In
> order to do so, we'll need to add a flag field to v4l2_event_source_change.
Ok a flag can be added with bitfields for reporting specific event type.
> The next __u32 right after the source_change field must thus be zeroed. I see
> two ways of doing so:
>
> - zeroing the whole data array before setting event-specific data
> - adding a reserved must-be-zeroed field to v4l2_event_source_change
>
> I like the former better as it's more generic, but we then need to ensure that
> all drivers zero the whole data field correctly. Adding a new
> v4l2_event_init() function would help with that.
>
Is that a good idea to have an init() function just for zeroing the data field?
If this is agreed upon, I can add this, but it can be easily missed
out by drivers.
Also how about the drivers already using the v4l2_event. Should we
update those drivers too with v4l2_event_init() ?
Regards
Arun
>> } u;
>> __u32 pending;
>
> --
> Regards,
>
> Laurent Pinchart
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] v4l: Add resolution change event.
Hi Arun,
Thank you for the patch.
On Monday 21 April 2014 14:56:01 Arun Kumar K wrote:
> From: Pawel Osciak
>
> This event indicates that the decoder has reached a point in the stream,
> at which the resolution changes. The userspace is expected to provide a new
> set of CAPTURE buffers for the new format before decoding can continue.
> The event can also be used for more generic events involving resolution
> or format changes at runtime for all kinds of video devices.
>
> Signed-off-by: Pawel Osciak
> Signed-off-by: Arun Kumar K
> ---
> .../DocBook/media/v4l/vidioc-subscribe-event.xml | 16
> include/uapi/linux/videodev2.h |6 ++
> 2 files changed, 22 insertions(+)
>
> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index
> 5c70b61..0aec831 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> @@ -155,6 +155,22 @@
>
>
>
> + V4L2_EVENT_SOURCE_CHANGE
> + 5
> +
> + This event is triggered when a resolution or format change
> +is detected during runtime by the video device. It can be a
> +runtime resolution change triggered by a video decoder or the
> +format change happening on an HDMI connector. Application may
> +need to reinitialize buffers before proceeding further.
> +
> + This event has a &v4l2-event-source-change; associated
> + with it. This has significance only for v4l2 subdevs where the
> + pad_num field will be updated with
> + the pad number on which the event is triggered.
> +
> +
> +
> V4L2_EVENT_PRIVATE_START
> 0x0800
> Base event number for driver-private events.
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 6ae7bbe..12e0614 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1733,6 +1733,7 @@ struct v4l2_streamparm {
> #define V4L2_EVENT_EOS 2
> #define V4L2_EVENT_CTRL 3
> #define V4L2_EVENT_FRAME_SYNC4
> +#define V4L2_EVENT_SOURCE_CHANGE 5
> #define V4L2_EVENT_PRIVATE_START 0x0800
>
> /* Payload for V4L2_EVENT_VSYNC */
> @@ -1764,12 +1765,17 @@ struct v4l2_event_frame_sync {
> __u32 frame_sequence;
> };
>
> +struct v4l2_event_source_change {
> + __u32 pad_num;
I would call the field just "pad",
> +};
> +
> struct v4l2_event {
> __u32 type;
> union {
> struct v4l2_event_vsync vsync;
> struct v4l2_event_ctrl ctrl;
> struct v4l2_event_frame_syncframe_sync;
> + struct v4l2_event_source_change source_change;
> __u8data[64];
This looks pretty good to me, but I'm a bit concerned about future
compatibility. We might need to report more information to userspace, and in
particular what has been changed at the source (resolution, format, ...). In
order to do so, we'll need to add a flag field to v4l2_event_source_change.
The next __u32 right after the source_change field must thus be zeroed. I see
two ways of doing so:
- zeroing the whole data array before setting event-specific data
- adding a reserved must-be-zeroed field to v4l2_event_source_change
I like the former better as it's more generic, but we then need to ensure that
all drivers zero the whole data field correctly. Adding a new
v4l2_event_init() function would help with that.
> } u;
> __u32 pending;
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] v4l: Add resolution change event.
Hi Pawel, On Mon, Apr 21, 2014 at 2:58 PM, Pawel Osciak wrote: > Arun, I think it'd better if you attributed this patch to yourself, I'm not > the author of it. I only wrote patch 2/2 in this series. > Thanks, > Pawel > > Sure I will change it. I kept your name as it was a rework of your patch from chrome tree. Will update the author name and post v3 if there are no other comments on this. Regards Arun -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
