[PATCH v3 0/2] Add resolution change event

[PATCH v3 0/2] Add resolution change event

[Arun Kumar K]

This patchset adds a source_change event to the v4l2-events.
This can be used for notifying the userspace about runtime
format changes happening on video nodes / pads like resolution
change in video decoder.

This version includes suggestions from Hans and Laurent
to make it more generic and to be used by all kinds of
video devices to notify userspace about runtime parameter
changes. The discussion can be seen here [1]

If this is accepted, I can send another series implementing
v4l2_event_init() for zeoring the data array addressing
compatibility issues as discussed in [1].

[1] https://patchwork.kernel.org/patch/4023131/

Changes from v2
---------------
- Event can be subscribed on specific pad / port as
  suggested by Hans.

Changes from v1
---------------
- Addressed review comments from Hans and Laurent
  https://patchwork.kernel.org/patch/4000951/

Arun Kumar K (1):
  [media] v4l: Add source change event

Pawel Osciak (1):
  [media] s5p-mfc: Add support for resolution change event

 Documentation/DocBook/media/v4l/vidioc-dqevent.xml |   32 +++++++++++++++++
 .../DocBook/media/v4l/vidioc-subscribe-event.xml   |   19 +++++++++++
 drivers/media/platform/s5p-mfc/s5p_mfc.c           |    7 ++++
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c       |    2 ++
 drivers/media/v4l2-core/v4l2-event.c               |   36 ++++++++++++++++++++
 include/media/v4l2-event.h                         |    4 +++
 include/uapi/linux/videodev2.h                     |    8 +++++
 7 files changed, 108 insertions(+)

-- 
1.7.9.5

[PATCH v3 1/2] [media] v4l: Add source change event

[Arun Kumar K]

This event indicates that the video device has encountered
a source parameter change during runtime. This can typically be a
resolution change detected by a video decoder OR a format change
detected by an HDMI connector.

This needs to be nofified to the userspace and the application may
be expected to reallocate buffers before proceeding. The application
can subscribe to events on a specific pad or input/output port which
it is interested in.

Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 Documentation/DocBook/media/v4l/vidioc-dqevent.xml |   32 +++++++++++++++++
 .../DocBook/media/v4l/vidioc-subscribe-event.xml   |   19 +++++++++++
 drivers/media/v4l2-core/v4l2-event.c               |   36 ++++++++++++++++++++
 include/media/v4l2-event.h                         |    4 +++
 include/uapi/linux/videodev2.h                     |    8 +++++
 5 files changed, 99 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
index 89891ad..6afabaa 100644
--- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
@@ -242,6 +242,22 @@
       </tgroup>
     </table>
 
+    <table frame="none" pgwide="1" id="v4l2-event-src-change">
+      <title>struct <structname>v4l2_event_src_change</structname></title>
+      <tgroup cols="3">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>changes</structfield></entry>
+	    <entry>
+	      A bitmask that tells what has changed. See <xref linkend="src-changes-flags" />.
+	    </entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
     <table pgwide="1" frame="none" id="changes-flags">
       <title>Changes</title>
       <tgroup cols="3">
@@ -270,6 +286,22 @@
 	</tbody>
       </tgroup>
     </table>
+
+    <table pgwide="1" frame="none" id="src-changes-flags">
+      <title>Source Changes</title>
+      <tgroup cols="3">
+	&cs-def;
+	<tbody valign="top">
+	  <row>
+	    <entry><constant>V4L2_EVENT_SRC_CH_RESOLUTION</constant></entry>
+	    <entry>0x0001</entry>
+	    <entry>This event gets triggered when a resolution change is
+	    detected at runtime. This can typically come from a video decoder.
+	    </entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
   </refsect1>
   <refsect1>
     &return-value;
diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
index 5c70b61..8012829 100644
--- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
@@ -155,6 +155,25 @@
 	    </entry>
 	  </row>
 	  <row>
+	    <entry><constant>V4L2_EVENT_SOURCE_CHANGE</constant></entry>
+	    <entry>5</entry>
+	    <entry>
+	      <para>This event is triggered when a 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.
+	       This event requires that the <structfield>id</structfield>
+               matches the pad/input/output index from which you want to
+	       receive events.</para>
+
+              <para>This event has a &v4l2-event-source-change; associated
+	      with it. The <structfield>changes</structfield> bitfield denotes
+	      what has changed for the subscribed pad. If multiple events
+	      occured before application could dequeue them, then the changes
+	      will have the ORed value of all the events generated.</para>
+	    </entry>
+	  </row>
+	  <row>
 	    <entry><constant>V4L2_EVENT_PRIVATE_START</constant></entry>
 	    <entry>0x08000000</entry>
 	    <entry>Base event number for driver-private events.</entry>
diff --git a/drivers/media/v4l2-core/v4l2-event.c b/drivers/media/v4l2-core/v4l2-event.c
index 86dcb54..8761aab 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -318,3 +318,39 @@ int v4l2_event_subdev_unsubscribe(struct v4l2_subdev *sd, struct v4l2_fh *fh,
 	return v4l2_event_unsubscribe(fh, sub);
 }
 EXPORT_SYMBOL_GPL(v4l2_event_subdev_unsubscribe);
+
+static 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;
+}
+
+static void v4l2_event_src_merge(const struct v4l2_event *old,
+				struct v4l2_event *new)
+{
+	new->u.src_change.changes |= old->u.src_change.changes;
+}
+
+static const struct v4l2_subscribed_event_ops v4l2_event_src_ch_ops = {
+	.replace = v4l2_event_src_replace,
+	.merge = v4l2_event_src_merge,
+};
+
+int v4l2_src_change_event_subscribe(struct v4l2_fh *fh,
+				const struct v4l2_event_subscription *sub)
+{
+	if (sub->type == V4L2_EVENT_SOURCE_CHANGE)
+		return v4l2_event_subscribe(fh, sub, 0, &v4l2_event_src_ch_ops);
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(v4l2_src_change_event_subscribe);
+
+int v4l2_src_change_event_subdev_subscribe(struct v4l2_subdev *sd,
+		struct v4l2_fh *fh, struct v4l2_event_subscription *sub)
+{
+	return v4l2_src_change_event_subscribe(fh, sub);
+}
+EXPORT_SYMBOL_GPL(v4l2_src_change_event_subdev_subscribe);
diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
index be05d01..1ab9045 100644
--- a/include/media/v4l2-event.h
+++ b/include/media/v4l2-event.h
@@ -132,4 +132,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
 void v4l2_event_unsubscribe_all(struct v4l2_fh *fh);
 int v4l2_event_subdev_unsubscribe(struct v4l2_subdev *sd, struct v4l2_fh *fh,
 				  struct v4l2_event_subscription *sub);
+int v4l2_src_change_event_subscribe(struct v4l2_fh *fh,
+				const struct v4l2_event_subscription *sub);
+int v4l2_src_change_event_subdev_subscribe(struct v4l2_subdev *sd,
+		struct v4l2_fh *fh, struct v4l2_event_subscription *sub);
 #endif /* V4L2_EVENT_H */
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index ea468ee..b923d91 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1765,6 +1765,7 @@ struct v4l2_streamparm {
 #define V4L2_EVENT_EOS				2
 #define V4L2_EVENT_CTRL				3
 #define V4L2_EVENT_FRAME_SYNC			4
+#define V4L2_EVENT_SOURCE_CHANGE		5
 #define V4L2_EVENT_PRIVATE_START		0x08000000
 
 /* Payload for V4L2_EVENT_VSYNC */
@@ -1796,12 +1797,19 @@ struct v4l2_event_frame_sync {
 	__u32 frame_sequence;
 };
 
+#define V4L2_EVENT_SRC_CH_RESOLUTION		(1 << 0)
+
+struct v4l2_event_src_change {
+	__u32 changes;
+};
+
 struct v4l2_event {
 	__u32				type;
 	union {
 		struct v4l2_event_vsync		vsync;
 		struct v4l2_event_ctrl		ctrl;
 		struct v4l2_event_frame_sync	frame_sync;
+		struct v4l2_event_src_change	src_change;
 		__u8				data[64];
 	} u;
 	__u32				pending;
-- 
1.7.9.5

[PATCH v3 2/2] [media] s5p-mfc: Add support for resolution change event

[Arun Kumar K]

From: Pawel Osciak <posciak@chromium.org>

When a resolution change point is reached, queue an event to signal the
userspace that a new set of buffers is required before decoding can
continue.

Signed-off-by: Pawel Osciak <posciak@chromium.org>
Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c     |    7 +++++++
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c |    2 ++
 2 files changed, 9 insertions(+)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 07c3d5e..c25a2b0 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -320,6 +320,7 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx *ctx,
 	struct s5p_mfc_buf *src_buf;
 	unsigned long flags;
 	unsigned int res_change;
+	struct v4l2_event ev;
 
 	dst_frame_status = s5p_mfc_hw_call(dev->mfc_ops, get_dspl_status, dev)
 				& S5P_FIMV_DEC_STATUS_DECODING_STATUS_MASK;
@@ -351,6 +352,12 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx *ctx,
 		if (ctx->state == MFCINST_RES_CHANGE_FLUSH) {
 			s5p_mfc_handle_frame_all_extracted(ctx);
 			ctx->state = MFCINST_RES_CHANGE_END;
+
+			memset(&ev, 0, sizeof(struct v4l2_event));
+			ev.type = V4L2_EVENT_SOURCE_CHANGE;
+			ev.u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION;
+			v4l2_event_queue_fh(&ctx->fh, &ev);
+
 			goto leave_handle_frame;
 		} else {
 			s5p_mfc_handle_frame_all_extracted(ctx);
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
index 4f94491..b383829 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
@@ -855,6 +855,8 @@ static int vidioc_subscribe_event(struct v4l2_fh *fh,
 	switch (sub->type) {
 	case V4L2_EVENT_EOS:
 		return v4l2_event_subscribe(fh, sub, 2, NULL);
+	case V4L2_EVENT_SOURCE_CHANGE:
+		return v4l2_src_change_event_subscribe(fh, sub);
 	default:
 		return -EINVAL;
 	}
-- 
1.7.9.5

Re: [PATCH v3 1/2] [media] v4l: Add source change event

[Laurent Pinchart]

Hi Arun,

Thank you for the patch. We're slowly getting there :-)

On Thursday 08 May 2014 17:19:15 Arun Kumar K wrote:
> This event indicates that the video device has encountered
> a source parameter change during runtime. This can typically be a
> resolution change detected by a video decoder OR a format change
> detected by an HDMI connector.
> 
> This needs to be nofified to the userspace and the application may
> be expected to reallocate buffers before proceeding. The application
> can subscribe to events on a specific pad or input/output port which
> it is interested in.
> 
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> ---
>  Documentation/DocBook/media/v4l/vidioc-dqevent.xml |   32 +++++++++++++++++
>  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |   19 +++++++++++
>  drivers/media/v4l2-core/v4l2-event.c               |   36 +++++++++++++++++
>  include/media/v4l2-event.h                         |    4 +++
>  include/uapi/linux/videodev2.h                     |    8 +++++
>  5 files changed, 99 insertions(+)
> 
> diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml index 89891ad..6afabaa
> 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> @@ -242,6 +242,22 @@
>        </tgroup>
>      </table>
> 
> +    <table frame="none" pgwide="1" id="v4l2-event-src-change">
> +      <title>struct <structname>v4l2_event_src_change</structname></title>
> +      <tgroup cols="3">
> +	&cs-str;
> +	<tbody valign="top">
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>changes</structfield></entry>
> +	    <entry>
> +	      A bitmask that tells what has changed. See <xref
> linkend="src-changes-flags" />. +	    </entry>
> +	  </row>
> +	</tbody>
> +      </tgroup>
> +    </table>
> +
>      <table pgwide="1" frame="none" id="changes-flags">
>        <title>Changes</title>
>        <tgroup cols="3">
> @@ -270,6 +286,22 @@
>  	</tbody>
>        </tgroup>
>      </table>
> +
> +    <table pgwide="1" frame="none" id="src-changes-flags">
> +      <title>Source Changes</title>
> +      <tgroup cols="3">
> +	&cs-def;
> +	<tbody valign="top">
> +	  <row>
> +	    <entry><constant>V4L2_EVENT_SRC_CH_RESOLUTION</constant></entry>
> +	    <entry>0x0001</entry>
> +	    <entry>This event gets triggered when a resolution change is
> +	    detected at runtime. This can typically come from a video decoder.
> +	    </entry>
> +	  </row>
> +	</tbody>
> +      </tgroup>
> +    </table>
>    </refsect1>
>    <refsect1>
>      &return-value;
> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index
> 5c70b61..8012829 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> @@ -155,6 +155,25 @@
>  	    </entry>
>  	  </row>
>  	  <row>
> +	    <entry><constant>V4L2_EVENT_SOURCE_CHANGE</constant></entry>
> +	    <entry>5</entry>
> +	    <entry>
> +	      <para>This event is triggered when a 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.
> +	       This event requires that the <structfield>id</structfield>
> +               matches the pad/input/output index from which you want to
> +	       receive events.</para>
> +
> +              <para>This event has a &v4l2-event-source-change; associated
> +	      with it. The <structfield>changes</structfield> bitfield denotes
> +	      what has changed for the subscribed pad. If multiple events
> +	      occured before application could dequeue them, then the changes
> +	      will have the ORed value of all the events generated.</para>
> +	    </entry>
> +	  </row>
> +	  <row>
>  	    <entry><constant>V4L2_EVENT_PRIVATE_START</constant></entry>
>  	    <entry>0x08000000</entry>
>  	    <entry>Base event number for driver-private events.</entry>
> diff --git a/drivers/media/v4l2-core/v4l2-event.c
> b/drivers/media/v4l2-core/v4l2-event.c index 86dcb54..8761aab 100644
> --- a/drivers/media/v4l2-core/v4l2-event.c
> +++ b/drivers/media/v4l2-core/v4l2-event.c
> @@ -318,3 +318,39 @@ int v4l2_event_subdev_unsubscribe(struct v4l2_subdev
> *sd, struct v4l2_fh *fh, return v4l2_event_unsubscribe(fh, sub);
>  }
>  EXPORT_SYMBOL_GPL(v4l2_event_subdev_unsubscribe);
> +
> +static 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;
> +}
> +
> +static void v4l2_event_src_merge(const struct v4l2_event *old,
> +				struct v4l2_event *new)
> +{
> +	new->u.src_change.changes |= old->u.src_change.changes;
> +}
> +
> +static const struct v4l2_subscribed_event_ops v4l2_event_src_ch_ops = {
> +	.replace = v4l2_event_src_replace,
> +	.merge = v4l2_event_src_merge,
> +};
> +
> +int v4l2_src_change_event_subscribe(struct v4l2_fh *fh,
> +				const struct v4l2_event_subscription *sub)
> +{
> +	if (sub->type == V4L2_EVENT_SOURCE_CHANGE)
> +		return v4l2_event_subscribe(fh, sub, 0, &v4l2_event_src_ch_ops);
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_src_change_event_subscribe);
> +
> +int v4l2_src_change_event_subdev_subscribe(struct v4l2_subdev *sd,
> +		struct v4l2_fh *fh, struct v4l2_event_subscription *sub)
> +{
> +	return v4l2_src_change_event_subscribe(fh, sub);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_src_change_event_subdev_subscribe);
> diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
> index be05d01..1ab9045 100644
> --- a/include/media/v4l2-event.h
> +++ b/include/media/v4l2-event.h
> @@ -132,4 +132,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
>  void v4l2_event_unsubscribe_all(struct v4l2_fh *fh);
>  int v4l2_event_subdev_unsubscribe(struct v4l2_subdev *sd, struct v4l2_fh
> *fh, struct v4l2_event_subscription *sub);
> +int v4l2_src_change_event_subscribe(struct v4l2_fh *fh,
> +				const struct v4l2_event_subscription *sub);
> +int v4l2_src_change_event_subdev_subscribe(struct v4l2_subdev *sd,
> +		struct v4l2_fh *fh, struct v4l2_event_subscription *sub);
>  #endif /* V4L2_EVENT_H */
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index ea468ee..b923d91 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1765,6 +1765,7 @@ struct v4l2_streamparm {
>  #define V4L2_EVENT_EOS				2
>  #define V4L2_EVENT_CTRL				3
>  #define V4L2_EVENT_FRAME_SYNC			4
> +#define V4L2_EVENT_SOURCE_CHANGE		5

My last concern is about the event name (and the name of the related 
structure). Either this event applies to inputs only, in which case the 
documentation shouldn't mention the output number as it does above, or the 
event applies to both inputs and outputs, in which case the name "source" is 
too restrictive. I'd be tempted to go for the second option, even though I 
have less use cases in mind on the output side.

>  #define V4L2_EVENT_PRIVATE_START		0x08000000
> 
>  /* Payload for V4L2_EVENT_VSYNC */
> @@ -1796,12 +1797,19 @@ struct v4l2_event_frame_sync {
>  	__u32 frame_sequence;
>  };
> 
> +#define V4L2_EVENT_SRC_CH_RESOLUTION		(1 << 0)
> +
> +struct v4l2_event_src_change {
> +	__u32 changes;
> +};
> +
>  struct v4l2_event {
>  	__u32				type;
>  	union {
>  		struct v4l2_event_vsync		vsync;
>  		struct v4l2_event_ctrl		ctrl;
>  		struct v4l2_event_frame_sync	frame_sync;
> +		struct v4l2_event_src_change	src_change;
>  		__u8				data[64];
>  	} u;
>  	__u32				pending;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 1/2] [media] v4l: Add source change event

[Hans Verkuil]

On 05/09/2014 03:09 PM, Laurent Pinchart wrote:
> Hi Arun,
> 
> Thank you for the patch. We're slowly getting there :-)
> 
> On Thursday 08 May 2014 17:19:15 Arun Kumar K wrote:
>> This event indicates that the video device has encountered
>> a source parameter change during runtime. This can typically be a
>> resolution change detected by a video decoder OR a format change
>> detected by an HDMI connector.
>>
>> This needs to be nofified to the userspace and the application may
>> be expected to reallocate buffers before proceeding. The application
>> can subscribe to events on a specific pad or input/output port which
>> it is interested in.
>>
>> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
>> ---
>>  Documentation/DocBook/media/v4l/vidioc-dqevent.xml |   32 +++++++++++++++++
>>  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |   19 +++++++++++
>>  drivers/media/v4l2-core/v4l2-event.c               |   36 +++++++++++++++++
>>  include/media/v4l2-event.h                         |    4 +++
>>  include/uapi/linux/videodev2.h                     |    8 +++++
>>  5 files changed, 99 insertions(+)
>>
>> diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
>> b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml index 89891ad..6afabaa
>> 100644
>> --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
>> +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
>> @@ -242,6 +242,22 @@
>>        </tgroup>
>>      </table>
>>
>> +    <table frame="none" pgwide="1" id="v4l2-event-src-change">
>> +      <title>struct <structname>v4l2_event_src_change</structname></title>
>> +      <tgroup cols="3">
>> +	&cs-str;
>> +	<tbody valign="top">
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>changes</structfield></entry>
>> +	    <entry>
>> +	      A bitmask that tells what has changed. See <xref
>> linkend="src-changes-flags" />. +	    </entry>
>> +	  </row>
>> +	</tbody>
>> +      </tgroup>
>> +    </table>
>> +
>>      <table pgwide="1" frame="none" id="changes-flags">
>>        <title>Changes</title>
>>        <tgroup cols="3">
>> @@ -270,6 +286,22 @@
>>  	</tbody>
>>        </tgroup>
>>      </table>
>> +
>> +    <table pgwide="1" frame="none" id="src-changes-flags">
>> +      <title>Source Changes</title>
>> +      <tgroup cols="3">
>> +	&cs-def;
>> +	<tbody valign="top">
>> +	  <row>
>> +	    <entry><constant>V4L2_EVENT_SRC_CH_RESOLUTION</constant></entry>
>> +	    <entry>0x0001</entry>
>> +	    <entry>This event gets triggered when a resolution change is
>> +	    detected at runtime. This can typically come from a video decoder.
>> +	    </entry>
>> +	  </row>
>> +	</tbody>
>> +      </tgroup>
>> +    </table>
>>    </refsect1>
>>    <refsect1>
>>      &return-value;
>> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index
>> 5c70b61..8012829 100644
>> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> @@ -155,6 +155,25 @@
>>  	    </entry>
>>  	  </row>
>>  	  <row>
>> +	    <entry><constant>V4L2_EVENT_SOURCE_CHANGE</constant></entry>
>> +	    <entry>5</entry>
>> +	    <entry>
>> +	      <para>This event is triggered when a 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.
>> +	       This event requires that the <structfield>id</structfield>
>> +               matches the pad/input/output index from which you want to
>> +	       receive events.</para>
>> +
>> +              <para>This event has a &v4l2-event-source-change; associated
>> +	      with it. The <structfield>changes</structfield> bitfield denotes
>> +	      what has changed for the subscribed pad. If multiple events
>> +	      occured before application could dequeue them, then the changes
>> +	      will have the ORed value of all the events generated.</para>
>> +	    </entry>
>> +	  </row>
>> +	  <row>
>>  	    <entry><constant>V4L2_EVENT_PRIVATE_START</constant></entry>
>>  	    <entry>0x08000000</entry>
>>  	    <entry>Base event number for driver-private events.</entry>
>> diff --git a/drivers/media/v4l2-core/v4l2-event.c
>> b/drivers/media/v4l2-core/v4l2-event.c index 86dcb54..8761aab 100644
>> --- a/drivers/media/v4l2-core/v4l2-event.c
>> +++ b/drivers/media/v4l2-core/v4l2-event.c
>> @@ -318,3 +318,39 @@ int v4l2_event_subdev_unsubscribe(struct v4l2_subdev
>> *sd, struct v4l2_fh *fh, return v4l2_event_unsubscribe(fh, sub);
>>  }
>>  EXPORT_SYMBOL_GPL(v4l2_event_subdev_unsubscribe);
>> +
>> +static 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;
>> +}
>> +
>> +static void v4l2_event_src_merge(const struct v4l2_event *old,
>> +				struct v4l2_event *new)
>> +{
>> +	new->u.src_change.changes |= old->u.src_change.changes;
>> +}
>> +
>> +static const struct v4l2_subscribed_event_ops v4l2_event_src_ch_ops = {
>> +	.replace = v4l2_event_src_replace,
>> +	.merge = v4l2_event_src_merge,
>> +};
>> +
>> +int v4l2_src_change_event_subscribe(struct v4l2_fh *fh,
>> +				const struct v4l2_event_subscription *sub)
>> +{
>> +	if (sub->type == V4L2_EVENT_SOURCE_CHANGE)
>> +		return v4l2_event_subscribe(fh, sub, 0, &v4l2_event_src_ch_ops);
>> +	return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_src_change_event_subscribe);
>> +
>> +int v4l2_src_change_event_subdev_subscribe(struct v4l2_subdev *sd,
>> +		struct v4l2_fh *fh, struct v4l2_event_subscription *sub)
>> +{
>> +	return v4l2_src_change_event_subscribe(fh, sub);
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_src_change_event_subdev_subscribe);
>> diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
>> index be05d01..1ab9045 100644
>> --- a/include/media/v4l2-event.h
>> +++ b/include/media/v4l2-event.h
>> @@ -132,4 +132,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
>>  void v4l2_event_unsubscribe_all(struct v4l2_fh *fh);
>>  int v4l2_event_subdev_unsubscribe(struct v4l2_subdev *sd, struct v4l2_fh
>> *fh, struct v4l2_event_subscription *sub);
>> +int v4l2_src_change_event_subscribe(struct v4l2_fh *fh,
>> +				const struct v4l2_event_subscription *sub);
>> +int v4l2_src_change_event_subdev_subscribe(struct v4l2_subdev *sd,
>> +		struct v4l2_fh *fh, struct v4l2_event_subscription *sub);
>>  #endif /* V4L2_EVENT_H */
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index ea468ee..b923d91 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -1765,6 +1765,7 @@ struct v4l2_streamparm {
>>  #define V4L2_EVENT_EOS				2
>>  #define V4L2_EVENT_CTRL				3
>>  #define V4L2_EVENT_FRAME_SYNC			4
>> +#define V4L2_EVENT_SOURCE_CHANGE		5
> 
> My last concern is about the event name (and the name of the related 
> structure). Either this event applies to inputs only, in which case the 
> documentation shouldn't mention the output number as it does above, or the 
> event applies to both inputs and outputs, in which case the name "source" is 
> too restrictive. I'd be tempted to go for the second option, even though I 
> have less use cases in mind on the output side.

I don't think it makes sense to support this for outputs (I think I originally
suggested that, but I didn't think that through). One practical problem with
that approach would be with m2m devices because that makes it impossible to
tell the difference between a change on the capture side and a change on the
output side, the other is that normally outputs do not change since you are in
control w.r.t. to the format/resolution, etc.

If there is a need for format change events on an output, then I would expect
to see a separate event for that.

Regards,

	Hans

> 
>>  #define V4L2_EVENT_PRIVATE_START		0x08000000
>>
>>  /* Payload for V4L2_EVENT_VSYNC */
>> @@ -1796,12 +1797,19 @@ struct v4l2_event_frame_sync {
>>  	__u32 frame_sequence;
>>  };
>>
>> +#define V4L2_EVENT_SRC_CH_RESOLUTION		(1 << 0)
>> +
>> +struct v4l2_event_src_change {
>> +	__u32 changes;
>> +};
>> +
>>  struct v4l2_event {
>>  	__u32				type;
>>  	union {
>>  		struct v4l2_event_vsync		vsync;
>>  		struct v4l2_event_ctrl		ctrl;
>>  		struct v4l2_event_frame_sync	frame_sync;
>> +		struct v4l2_event_src_change	src_change;
>>  		__u8				data[64];
>>  	} u;
>>  	__u32				pending;
> 

Re: [PATCH v3 1/2] [media] v4l: Add source change event

[Arun Kumar K]

Hi Hans, Laurent,

On 05/09/14 18:45, Hans Verkuil wrote:
> On 05/09/2014 03:09 PM, Laurent Pinchart wrote:
>> Hi Arun,
>>
>> Thank you for the patch. We're slowly getting there :-)
>>
>> On Thursday 08 May 2014 17:19:15 Arun Kumar K wrote:
>>> This event indicates that the video device has encountered
>>> a source parameter change during runtime. This can typically be a
>>> resolution change detected by a video decoder OR a format change
>>> detected by an HDMI connector.
>>>
>>> This needs to be nofified to the userspace and the application may
>>> be expected to reallocate buffers before proceeding. The application
>>> can subscribe to events on a specific pad or input/output port which
>>> it is interested in.
>>>
>>> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
>>> ---
>>>  Documentation/DocBook/media/v4l/vidioc-dqevent.xml |   32 +++++++++++++++++
>>>  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |   19 +++++++++++
>>>  drivers/media/v4l2-core/v4l2-event.c               |   36 +++++++++++++++++
>>>  include/media/v4l2-event.h                         |    4 +++
>>>  include/uapi/linux/videodev2.h                     |    8 +++++
>>>  5 files changed, 99 insertions(+)
>>>
>>> diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
>>> b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml index 89891ad..6afabaa
>>> 100644
>>> --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
>>> +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
>>> @@ -242,6 +242,22 @@
>>>        </tgroup>
>>>      </table>
>>>
>>> +    <table frame="none" pgwide="1" id="v4l2-event-src-change">
>>> +      <title>struct <structname>v4l2_event_src_change</structname></title>
>>> +      <tgroup cols="3">
>>> +	&cs-str;
>>> +	<tbody valign="top">
>>> +	  <row>
>>> +	    <entry>__u32</entry>
>>> +	    <entry><structfield>changes</structfield></entry>
>>> +	    <entry>
>>> +	      A bitmask that tells what has changed. See <xref
>>> linkend="src-changes-flags" />. +	    </entry>
>>> +	  </row>
>>> +	</tbody>
>>> +      </tgroup>
>>> +    </table>
>>> +
>>>      <table pgwide="1" frame="none" id="changes-flags">
>>>        <title>Changes</title>
>>>        <tgroup cols="3">
>>> @@ -270,6 +286,22 @@
>>>  	</tbody>
>>>        </tgroup>
>>>      </table>
>>> +
>>> +    <table pgwide="1" frame="none" id="src-changes-flags">
>>> +      <title>Source Changes</title>
>>> +      <tgroup cols="3">
>>> +	&cs-def;
>>> +	<tbody valign="top">
>>> +	  <row>
>>> +	    <entry><constant>V4L2_EVENT_SRC_CH_RESOLUTION</constant></entry>
>>> +	    <entry>0x0001</entry>
>>> +	    <entry>This event gets triggered when a resolution change is
>>> +	    detected at runtime. This can typically come from a video decoder.
>>> +	    </entry>
>>> +	  </row>
>>> +	</tbody>
>>> +      </tgroup>
>>> +    </table>
>>>    </refsect1>
>>>    <refsect1>
>>>      &return-value;
>>> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>>> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index
>>> 5c70b61..8012829 100644
>>> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>>> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>>> @@ -155,6 +155,25 @@
>>>  	    </entry>
>>>  	  </row>
>>>  	  <row>
>>> +	    <entry><constant>V4L2_EVENT_SOURCE_CHANGE</constant></entry>
>>> +	    <entry>5</entry>
>>> +	    <entry>
>>> +	      <para>This event is triggered when a 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.
>>> +	       This event requires that the <structfield>id</structfield>
>>> +               matches the pad/input/output index from which you want to
>>> +	       receive events.</para>
>>> +
>>> +              <para>This event has a &v4l2-event-source-change; associated
>>> +	      with it. The <structfield>changes</structfield> bitfield denotes
>>> +	      what has changed for the subscribed pad. If multiple events
>>> +	      occured before application could dequeue them, then the changes
>>> +	      will have the ORed value of all the events generated.</para>
>>> +	    </entry>
>>> +	  </row>
>>> +	  <row>
>>>  	    <entry><constant>V4L2_EVENT_PRIVATE_START</constant></entry>
>>>  	    <entry>0x08000000</entry>
>>>  	    <entry>Base event number for driver-private events.</entry>
>>> diff --git a/drivers/media/v4l2-core/v4l2-event.c
>>> b/drivers/media/v4l2-core/v4l2-event.c index 86dcb54..8761aab 100644
>>> --- a/drivers/media/v4l2-core/v4l2-event.c
>>> +++ b/drivers/media/v4l2-core/v4l2-event.c
>>> @@ -318,3 +318,39 @@ int v4l2_event_subdev_unsubscribe(struct v4l2_subdev
>>> *sd, struct v4l2_fh *fh, return v4l2_event_unsubscribe(fh, sub);
>>>  }
>>>  EXPORT_SYMBOL_GPL(v4l2_event_subdev_unsubscribe);
>>> +
>>> +static 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;
>>> +}
>>> +
>>> +static void v4l2_event_src_merge(const struct v4l2_event *old,
>>> +				struct v4l2_event *new)
>>> +{
>>> +	new->u.src_change.changes |= old->u.src_change.changes;
>>> +}
>>> +
>>> +static const struct v4l2_subscribed_event_ops v4l2_event_src_ch_ops = {
>>> +	.replace = v4l2_event_src_replace,
>>> +	.merge = v4l2_event_src_merge,
>>> +};
>>> +
>>> +int v4l2_src_change_event_subscribe(struct v4l2_fh *fh,
>>> +				const struct v4l2_event_subscription *sub)
>>> +{
>>> +	if (sub->type == V4L2_EVENT_SOURCE_CHANGE)
>>> +		return v4l2_event_subscribe(fh, sub, 0, &v4l2_event_src_ch_ops);
>>> +	return -EINVAL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(v4l2_src_change_event_subscribe);
>>> +
>>> +int v4l2_src_change_event_subdev_subscribe(struct v4l2_subdev *sd,
>>> +		struct v4l2_fh *fh, struct v4l2_event_subscription *sub)
>>> +{
>>> +	return v4l2_src_change_event_subscribe(fh, sub);
>>> +}
>>> +EXPORT_SYMBOL_GPL(v4l2_src_change_event_subdev_subscribe);
>>> diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
>>> index be05d01..1ab9045 100644
>>> --- a/include/media/v4l2-event.h
>>> +++ b/include/media/v4l2-event.h
>>> @@ -132,4 +132,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
>>>  void v4l2_event_unsubscribe_all(struct v4l2_fh *fh);
>>>  int v4l2_event_subdev_unsubscribe(struct v4l2_subdev *sd, struct v4l2_fh
>>> *fh, struct v4l2_event_subscription *sub);
>>> +int v4l2_src_change_event_subscribe(struct v4l2_fh *fh,
>>> +				const struct v4l2_event_subscription *sub);
>>> +int v4l2_src_change_event_subdev_subscribe(struct v4l2_subdev *sd,
>>> +		struct v4l2_fh *fh, struct v4l2_event_subscription *sub);
>>>  #endif /* V4L2_EVENT_H */
>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>> index ea468ee..b923d91 100644
>>> --- a/include/uapi/linux/videodev2.h
>>> +++ b/include/uapi/linux/videodev2.h
>>> @@ -1765,6 +1765,7 @@ struct v4l2_streamparm {
>>>  #define V4L2_EVENT_EOS				2
>>>  #define V4L2_EVENT_CTRL				3
>>>  #define V4L2_EVENT_FRAME_SYNC			4
>>> +#define V4L2_EVENT_SOURCE_CHANGE		5
>>
>> My last concern is about the event name (and the name of the related 
>> structure). Either this event applies to inputs only, in which case the 
>> documentation shouldn't mention the output number as it does above, or the 
>> event applies to both inputs and outputs, in which case the name "source" is 
>> too restrictive. I'd be tempted to go for the second option, even though I 
>> have less use cases in mind on the output side.
> 
> I don't think it makes sense to support this for outputs (I think I originally
> suggested that, but I didn't think that through). One practical problem with
> that approach would be with m2m devices because that makes it impossible to
> tell the difference between a change on the capture side and a change on the
> output side, the other is that normally outputs do not change since you are in
> control w.r.t. to the format/resolution, etc.
> 
> If there is a need for format change events on an output, then I would expect
> to see a separate event for that.
> 

Ok. I will modify the documentation to mention only about pad / input
index and keep the same name.

Regards
Arun

> Regards,
> 
> 	Hans
> 
>>
>>>  #define V4L2_EVENT_PRIVATE_START		0x08000000
>>>
>>>  /* Payload for V4L2_EVENT_VSYNC */
>>> @@ -1796,12 +1797,19 @@ struct v4l2_event_frame_sync {
>>>  	__u32 frame_sequence;
>>>  };
>>>
>>> +#define V4L2_EVENT_SRC_CH_RESOLUTION		(1 << 0)
>>> +
>>> +struct v4l2_event_src_change {
>>> +	__u32 changes;
>>> +};
>>> +
>>>  struct v4l2_event {
>>>  	__u32				type;
>>>  	union {
>>>  		struct v4l2_event_vsync		vsync;
>>>  		struct v4l2_event_ctrl		ctrl;
>>>  		struct v4l2_event_frame_sync	frame_sync;
>>> +		struct v4l2_event_src_change	src_change;
>>>  		__u8				data[64];
>>>  	} u;
>>>  	__u32				pending;
>>
>