public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] vb2: Allow STREAMOFF for io emulator
@ 2013-10-04 13:49 Ricardo Ribalda Delgado
  2013-10-04 14:09 ` Hans Verkuil
  2013-10-08  7:22 ` Marek Szyprowski
  0 siblings, 2 replies; 6+ messages in thread
From: Ricardo Ribalda Delgado @ 2013-10-04 13:49 UTC (permalink / raw)
  To: Pawel Osciak, Marek Szyprowski, Kyungmin Park,
	Mauro Carvalho Chehab, linux-media, linux-kernel
  Cc: Ricardo Ribalda Delgado

A video device opened and streaming in io emulator mode can only stop
streamming if its file descriptor is closed.

There are some parameters that can only be changed if the device is not
streaming. Also, the power consumption of a device streaming could be
different than one not streaming.

With this patch a video device opened in io emulator can be stopped on
demand.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 9fc4bab..097fba8 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1686,6 +1686,7 @@ int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
 }
 EXPORT_SYMBOL_GPL(vb2_streamon);
 
+static int __vb2_cleanup_fileio(struct vb2_queue *q);
 
 /**
  * vb2_streamoff - stop streaming
@@ -1704,11 +1705,6 @@ EXPORT_SYMBOL_GPL(vb2_streamon);
  */
 int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
 {
-	if (q->fileio) {
-		dprintk(1, "streamoff: file io in progress\n");
-		return -EBUSY;
-	}
-
 	if (type != q->type) {
 		dprintk(1, "streamoff: invalid stream type\n");
 		return -EINVAL;
@@ -1719,6 +1715,11 @@ int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
 		return -EINVAL;
 	}
 
+	if (q->fileio) {
+		__vb2_cleanup_fileio(q);
+		return 0;
+	}
+
 	/*
 	 * Cancel will pause streaming and remove all buffers from the driver
 	 * and videobuf, effectively returning control over them to userspace.
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] vb2: Allow STREAMOFF for io emulator
  2013-10-04 13:49 [PATCH] vb2: Allow STREAMOFF for io emulator Ricardo Ribalda Delgado
@ 2013-10-04 14:09 ` Hans Verkuil
  2013-10-04 14:16   ` Ricardo Ribalda Delgado
  2013-10-08  7:22 ` Marek Szyprowski
  1 sibling, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2013-10-04 14:09 UTC (permalink / raw)
  To: Ricardo Ribalda Delgado
  Cc: Pawel Osciak, Marek Szyprowski, Kyungmin Park,
	Mauro Carvalho Chehab, linux-media, linux-kernel

Hi Ricardo,

On 10/04/2013 03:49 PM, Ricardo Ribalda Delgado wrote:
> A video device opened and streaming in io emulator mode can only stop
> streamming if its file descriptor is closed.
> 
> There are some parameters that can only be changed if the device is not
> streaming. Also, the power consumption of a device streaming could be
> different than one not streaming.
> 
> With this patch a video device opened in io emulator can be stopped on
> demand.

Why would you want this? If you can call STREAMOFF, why not use stream I/O
all the way? That's much more efficient than read() anyway.

Unless there is a very good use-case, I don't see a good reason for mixing
file I/O with streaming I/O ioctls.

Regards,

	Hans

> 
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 9fc4bab..097fba8 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1686,6 +1686,7 @@ int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>  }
>  EXPORT_SYMBOL_GPL(vb2_streamon);
>  
> +static int __vb2_cleanup_fileio(struct vb2_queue *q);
>  
>  /**
>   * vb2_streamoff - stop streaming
> @@ -1704,11 +1705,6 @@ EXPORT_SYMBOL_GPL(vb2_streamon);
>   */
>  int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>  {
> -	if (q->fileio) {
> -		dprintk(1, "streamoff: file io in progress\n");
> -		return -EBUSY;
> -	}
> -
>  	if (type != q->type) {
>  		dprintk(1, "streamoff: invalid stream type\n");
>  		return -EINVAL;
> @@ -1719,6 +1715,11 @@ int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>  		return -EINVAL;
>  	}
>  
> +	if (q->fileio) {
> +		__vb2_cleanup_fileio(q);
> +		return 0;
> +	}
> +
>  	/*
>  	 * Cancel will pause streaming and remove all buffers from the driver
>  	 * and videobuf, effectively returning control over them to userspace.
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] vb2: Allow STREAMOFF for io emulator
  2013-10-04 14:09 ` Hans Verkuil
@ 2013-10-04 14:16   ` Ricardo Ribalda Delgado
  0 siblings, 0 replies; 6+ messages in thread
From: Ricardo Ribalda Delgado @ 2013-10-04 14:16 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Pawel Osciak, Marek Szyprowski, Kyungmin Park,
	Mauro Carvalho Chehab, linux-media, LKML

Hello Hans

I am implementing a test application for our camera, think of
v4l2-compliance but for testing the hardware (average of pixels,
rotation...) . I am implementing it using python (because of numpy and
matplotlib).

I dont really care about perferomance, I only care about the data
correctness, so the fileio fits perfectly my needs.

On the fileio api we dont have a way to tell the camera to stop, this
was an attempt to solve this "issue". But if it is only an issue for
me we can forget about it :).

BTW, do you know about a complete v4l2 library for python? I am using
https://pypi.python.org/pypi/v4l2 , but it is quite old.

Thanks!


On Fri, Oct 4, 2013 at 4:09 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Hi Ricardo,
>
> On 10/04/2013 03:49 PM, Ricardo Ribalda Delgado wrote:
>> A video device opened and streaming in io emulator mode can only stop
>> streamming if its file descriptor is closed.
>>
>> There are some parameters that can only be changed if the device is not
>> streaming. Also, the power consumption of a device streaming could be
>> different than one not streaming.
>>
>> With this patch a video device opened in io emulator can be stopped on
>> demand.
>
> Why would you want this? If you can call STREAMOFF, why not use stream I/O
> all the way? That's much more efficient than read() anyway.
>
> Unless there is a very good use-case, I don't see a good reason for mixing
> file I/O with streaming I/O ioctls.
>
> Regards,
>
>         Hans
>
>>
>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>> ---
>>  drivers/media/v4l2-core/videobuf2-core.c | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
>> index 9fc4bab..097fba8 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -1686,6 +1686,7 @@ int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>>  }
>>  EXPORT_SYMBOL_GPL(vb2_streamon);
>>
>> +static int __vb2_cleanup_fileio(struct vb2_queue *q);
>>
>>  /**
>>   * vb2_streamoff - stop streaming
>> @@ -1704,11 +1705,6 @@ EXPORT_SYMBOL_GPL(vb2_streamon);
>>   */
>>  int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>>  {
>> -     if (q->fileio) {
>> -             dprintk(1, "streamoff: file io in progress\n");
>> -             return -EBUSY;
>> -     }
>> -
>>       if (type != q->type) {
>>               dprintk(1, "streamoff: invalid stream type\n");
>>               return -EINVAL;
>> @@ -1719,6 +1715,11 @@ int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>>               return -EINVAL;
>>       }
>>
>> +     if (q->fileio) {
>> +             __vb2_cleanup_fileio(q);
>> +             return 0;
>> +     }
>> +
>>       /*
>>        * Cancel will pause streaming and remove all buffers from the driver
>>        * and videobuf, effectively returning control over them to userspace.
>>
>



-- 
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] vb2: Allow STREAMOFF for io emulator
  2013-10-04 13:49 [PATCH] vb2: Allow STREAMOFF for io emulator Ricardo Ribalda Delgado
  2013-10-04 14:09 ` Hans Verkuil
@ 2013-10-08  7:22 ` Marek Szyprowski
  2013-10-08  7:58   ` Ricardo Ribalda Delgado
  1 sibling, 1 reply; 6+ messages in thread
From: Marek Szyprowski @ 2013-10-08  7:22 UTC (permalink / raw)
  To: Ricardo Ribalda Delgado
  Cc: Pawel Osciak, Kyungmin Park, Mauro Carvalho Chehab, linux-media,
	linux-kernel

Hello,

On 2013-10-04 15:49, Ricardo Ribalda Delgado wrote:
> A video device opened and streaming in io emulator mode can only stop
> streamming if its file descriptor is closed.
>
> There are some parameters that can only be changed if the device is not
> streaming. Also, the power consumption of a device streaming could be
> different than one not streaming.
>
> With this patch a video device opened in io emulator can be stopped on
> demand.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Read/write-based io mode must not be mixed with ioctrl-based IO, so I 
really cannot accept this patch. Check V4L2 documentation for more details.

> ---
>   drivers/media/v4l2-core/videobuf2-core.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 9fc4bab..097fba8 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1686,6 +1686,7 @@ int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>   }
>   EXPORT_SYMBOL_GPL(vb2_streamon);
>   
> +static int __vb2_cleanup_fileio(struct vb2_queue *q);
>   
>   /**
>    * vb2_streamoff - stop streaming
> @@ -1704,11 +1705,6 @@ EXPORT_SYMBOL_GPL(vb2_streamon);
>    */
>   int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>   {
> -	if (q->fileio) {
> -		dprintk(1, "streamoff: file io in progress\n");
> -		return -EBUSY;
> -	}
> -
>   	if (type != q->type) {
>   		dprintk(1, "streamoff: invalid stream type\n");
>   		return -EINVAL;
> @@ -1719,6 +1715,11 @@ int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>   		return -EINVAL;
>   	}
>   
> +	if (q->fileio) {
> +		__vb2_cleanup_fileio(q);
> +		return 0;
> +	}
> +
>   	/*
>   	 * Cancel will pause streaming and remove all buffers from the driver
>   	 * and videobuf, effectively returning control over them to userspace.

Best regards
-- 
Marek Szyprowski
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] vb2: Allow STREAMOFF for io emulator
  2013-10-08  7:22 ` Marek Szyprowski
@ 2013-10-08  7:58   ` Ricardo Ribalda Delgado
  2013-10-08 10:00     ` Marek Szyprowski
  0 siblings, 1 reply; 6+ messages in thread
From: Ricardo Ribalda Delgado @ 2013-10-08  7:58 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Pawel Osciak, Kyungmin Park, Mauro Carvalho Chehab, linux-media,
	LKML

Hi Marek

Thanks for your comments. I was just trying to find a way to stop
streaming while in read/write mode without having to close the
descriptor. I thought reusing streamoff was more clever than creating
a new ioctl.

Thanks!

On Tue, Oct 8, 2013 at 9:22 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Hello,
>
>
> On 2013-10-04 15:49, Ricardo Ribalda Delgado wrote:
>>
>> A video device opened and streaming in io emulator mode can only stop
>> streamming if its file descriptor is closed.
>>
>> There are some parameters that can only be changed if the device is not
>> streaming. Also, the power consumption of a device streaming could be
>> different than one not streaming.
>>
>> With this patch a video device opened in io emulator can be stopped on
>> demand.
>>
>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>
>
> Read/write-based io mode must not be mixed with ioctrl-based IO, so I really
> cannot accept this patch. Check V4L2 documentation for more details.
>
>
>> ---
>>   drivers/media/v4l2-core/videobuf2-core.c | 11 ++++++-----
>>   1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index 9fc4bab..097fba8 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -1686,6 +1686,7 @@ int vb2_streamon(struct vb2_queue *q, enum
>> v4l2_buf_type type)
>>   }
>>   EXPORT_SYMBOL_GPL(vb2_streamon);
>>   +static int __vb2_cleanup_fileio(struct vb2_queue *q);
>>     /**
>>    * vb2_streamoff - stop streaming
>> @@ -1704,11 +1705,6 @@ EXPORT_SYMBOL_GPL(vb2_streamon);
>>    */
>>   int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>>   {
>> -       if (q->fileio) {
>> -               dprintk(1, "streamoff: file io in progress\n");
>> -               return -EBUSY;
>> -       }
>> -
>>         if (type != q->type) {
>>                 dprintk(1, "streamoff: invalid stream type\n");
>>                 return -EINVAL;
>> @@ -1719,6 +1715,11 @@ int vb2_streamoff(struct vb2_queue *q, enum
>> v4l2_buf_type type)
>>                 return -EINVAL;
>>         }
>>   +     if (q->fileio) {
>> +               __vb2_cleanup_fileio(q);
>> +               return 0;
>> +       }
>> +
>>         /*
>>          * Cancel will pause streaming and remove all buffers from the
>> driver
>>          * and videobuf, effectively returning control over them to
>> userspace.
>
>
> Best regards
> --
> Marek Szyprowski
> Samsung R&D Institute Poland
>



-- 
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] vb2: Allow STREAMOFF for io emulator
  2013-10-08  7:58   ` Ricardo Ribalda Delgado
@ 2013-10-08 10:00     ` Marek Szyprowski
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Szyprowski @ 2013-10-08 10:00 UTC (permalink / raw)
  To: Ricardo Ribalda Delgado
  Cc: Pawel Osciak, Kyungmin Park, Mauro Carvalho Chehab, linux-media,
	LKML

Hi Racardo,

On 2013-10-08 09:58, Ricardo Ribalda Delgado wrote:
> Hi Marek
>
> Thanks for your comments. I was just trying to find a way to stop
> streaming while in read/write mode without having to close the
> descriptor. I thought reusing streamoff was more clever than creating
> a new ioctl.

Read()/write() mode is mainly designed for old and legacy applications. 
Those applications assume that the only reliable way to stop streaming 
is to close the fd. New tools should use libv4l and ioctl-based api.

Best regards
-- 
Marek Szyprowski
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-10-08 10:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-04 13:49 [PATCH] vb2: Allow STREAMOFF for io emulator Ricardo Ribalda Delgado
2013-10-04 14:09 ` Hans Verkuil
2013-10-04 14:16   ` Ricardo Ribalda Delgado
2013-10-08  7:22 ` Marek Szyprowski
2013-10-08  7:58   ` Ricardo Ribalda Delgado
2013-10-08 10:00     ` Marek Szyprowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox