[PATCH 1/2] v4l: Add resolution change event.

[PATCH 1/2] v4l: Add resolution change event.

[Arun Kumar K]

From: Pawel Osciak <posciak@chromium.org>

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.

Signed-off-by: Pawel Osciak <posciak@chromium.org>
Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 .../DocBook/media/v4l/vidioc-subscribe-event.xml   |    8 ++++++++
 include/uapi/linux/videodev2.h                     |    1 +
 2 files changed, 9 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
index 5c70b61..d848628 100644
--- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
@@ -155,6 +155,14 @@
 	    </entry>
 	  </row>
 	  <row>
+	    <entry><constant>V4L2_EVENT_RESOLUTION_CHANGE</constant></entry>
+	    <entry>5</entry>
+	    <entry>This event is triggered when a resolution change is detected
+	    during runtime by the video decoder. Application may need to
+	    reinitialize buffers before proceeding further.
+	    </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/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 6ae7bbe..58488b7 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_RESOLUTION_CHANGE		5
 #define V4L2_EVENT_PRIVATE_START		0x08000000
 
 /* Payload for V4L2_EVENT_VSYNC */
-- 
1.7.9.5

[PATCH 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     |    6 ++++++
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c |    2 ++
 2 files changed, 8 insertions(+)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 4ee5a02..2efa758 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -308,6 +308,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;
@@ -339,6 +340,11 @@ 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_RESOLUTION_CHANGE;
+			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 4586186..dd0ec6f 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
@@ -851,6 +851,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_RESOLUTION_CHANGE:
+		return v4l2_event_subscribe(fh, sub, 2, NULL);
 	default:
 		return -EINVAL;
 	}
-- 
1.7.9.5

Re: [PATCH 1/2] v4l: Add resolution change event.

[Laurent Pinchart]

Hi Arun,

Thank you for the patch.
On Wednesday 16 April 2014 18:29:21 Arun Kumar K wrote:
> From: Pawel Osciak <posciak@chromium.org>
> 
> 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.
> 
> Signed-off-by: Pawel Osciak <posciak@chromium.org>
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> ---
>  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |    8 ++++++++
>  include/uapi/linux/videodev2.h                     |    1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index
> 5c70b61..d848628 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> @@ -155,6 +155,14 @@
>  	    </entry>
>  	  </row>
>  	  <row>
> +	    <entry><constant>V4L2_EVENT_RESOLUTION_CHANGE</constant></entry>
> +	    <entry>5</entry>
> +	    <entry>This event is triggered when a resolution change is detected
> +	    during runtime by the video decoder. Application may need to
> +	    reinitialize buffers before proceeding further.
> +	    </entry>
> +	  </row>

Would it make sense to report the new resolution in the event data ? I suppose 
it might not be available in all cases though. If we can't report it, would it 
make sense to document how applications should proceed to retrieve it ?

A similar resolution change event might be useful on subdevs, in which case we 
would need to add a pad number to the event data. We could possibly leave that 
for later, but it would be worth considering the problem already.

> +	  <row>
>  	    <entry><constant>V4L2_EVENT_PRIVATE_START</constant></entry>
>  	    <entry>0x08000000</entry>
>  	    <entry>Base event number for driver-private events.</entry>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 6ae7bbe..58488b7 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_RESOLUTION_CHANGE		5
>  #define V4L2_EVENT_PRIVATE_START		0x08000000
> 
>  /* Payload for V4L2_EVENT_VSYNC */

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] v4l: Add resolution change event.

[Hans Verkuil]

On 04/16/2014 04:09 PM, Laurent Pinchart wrote:
> Hi Arun,
> 
> Thank you for the patch.
> On Wednesday 16 April 2014 18:29:21 Arun Kumar K wrote:
>> From: Pawel Osciak <posciak@chromium.org>
>>
>> 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.
>>
>> Signed-off-by: Pawel Osciak <posciak@chromium.org>
>> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
>> ---
>>  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |    8 ++++++++
>>  include/uapi/linux/videodev2.h                     |    1 +
>>  2 files changed, 9 insertions(+)
>>
>> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index
>> 5c70b61..d848628 100644
>> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> @@ -155,6 +155,14 @@
>>  	    </entry>
>>  	  </row>
>>  	  <row>
>> +	    <entry><constant>V4L2_EVENT_RESOLUTION_CHANGE</constant></entry>
>> +	    <entry>5</entry>
>> +	    <entry>This event is triggered when a resolution change is detected
>> +	    during runtime by the video decoder. Application may need to
>> +	    reinitialize buffers before proceeding further.
>> +	    </entry>
>> +	  </row>
> 
> Would it make sense to report the new resolution in the event data ? I suppose 
> it might not be available in all cases though. If we can't report it, would it 
> make sense to document how applications should proceed to retrieve it ?

I wouldn't report that. We played with this in Cisco, and in the end you just
want to know something changed and you can take it from there. Besides, what
constitutes a 'resolution' change? If my HDMI input switches from 720p60 to
720p30 the resolution stays the same, but I most definitely have to get the new
timings. 

So I would call the event something different: EVENT_SOURCE_CHANGE or something
like that.

Getting the new timings is done through QUERYSTD or QUERY_DV_TIMINGS.

> A similar resolution change event might be useful on subdevs, in which case we 
> would need to add a pad number to the event data. We could possibly leave that 
> for later, but it would be worth considering the problem already.

Actually, I would add that right away. That's some thing that the adv7604
driver can implement right away: it has multiple inputs and it can detect
when something is plugged in or unplugged.

Regards,

	Hans

> 
>> +	  <row>
>>  	    <entry><constant>V4L2_EVENT_PRIVATE_START</constant></entry>
>>  	    <entry>0x08000000</entry>
>>  	    <entry>Base event number for driver-private events.</entry>
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index 6ae7bbe..58488b7 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_RESOLUTION_CHANGE		5
>>  #define V4L2_EVENT_PRIVATE_START		0x08000000
>>
>>  /* Payload for V4L2_EVENT_VSYNC */
> 

Re: [PATCH 1/2] v4l: Add resolution change event.

[Arun Kumar K]

Hi Laurent and Hans,

Thank you for the review.

On Wed, Apr 16, 2014 at 7:46 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 04/16/2014 04:09 PM, Laurent Pinchart wrote:
>> Hi Arun,
>>
>> Thank you for the patch.
>> On Wednesday 16 April 2014 18:29:21 Arun Kumar K wrote:
>>> From: Pawel Osciak <posciak@chromium.org>
>>>
>>> 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.
>>>
>>> Signed-off-by: Pawel Osciak <posciak@chromium.org>
>>> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
>>> ---
>>>  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |    8 ++++++++
>>>  include/uapi/linux/videodev2.h                     |    1 +
>>>  2 files changed, 9 insertions(+)
>>>
>>> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>>> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index
>>> 5c70b61..d848628 100644
>>> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>>> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>>> @@ -155,6 +155,14 @@
>>>          </entry>
>>>        </row>
>>>        <row>
>>> +        <entry><constant>V4L2_EVENT_RESOLUTION_CHANGE</constant></entry>
>>> +        <entry>5</entry>
>>> +        <entry>This event is triggered when a resolution change is detected
>>> +        during runtime by the video decoder. Application may need to
>>> +        reinitialize buffers before proceeding further.
>>> +        </entry>
>>> +      </row>
>>
>> Would it make sense to report the new resolution in the event data ? I suppose
>> it might not be available in all cases though. If we can't report it, would it
>> make sense to document how applications should proceed to retrieve it ?
>
> I wouldn't report that. We played with this in Cisco, and in the end you just
> want to know something changed and you can take it from there. Besides, what
> constitutes a 'resolution' change? If my HDMI input switches from 720p60 to
> 720p30 the resolution stays the same, but I most definitely have to get the new
> timings.
>
> So I would call the event something different: EVENT_SOURCE_CHANGE or something
> like that.
>
> Getting the new timings is done through QUERYSTD or QUERY_DV_TIMINGS.
>

Ok will use the name V4L2_EVENT_SOURCE_CHANGE and update description
to reflect the generic usecase (not just for video decoders).

>> A similar resolution change event might be useful on subdevs, in which case we
>> would need to add a pad number to the event data. We could possibly leave that
>> for later, but it would be worth considering the problem already.
>
> Actually, I would add that right away. That's some thing that the adv7604
> driver can implement right away: it has multiple inputs and it can detect
> when something is plugged in or unplugged.
>

Ok will add support for mentioning pad number in event data.

Regards
Arun