diff mbox series

au0828: stop video streaming only when last user stops

Message ID 6add9687-d261-889f-af30-635b065a49c9@xs4all.nl (mailing list archive)
State New, archived
Headers show
Series au0828: stop video streaming only when last user stops | expand

Commit Message

Hans Verkuil April 1, 2019, 11:18 a.m. UTC
If the application was streaming from both videoX and vbiX, and streaming
from videoX was stopped, then the vbi streaming also stopped.

The cause being that stop_streaming for video stopped the subdevs as well,
instead of only doing that if dev->streaming_users reached 0.

au0828_stop_vbi_streaming was also wrong since it didn't stop the subdevs
at all when dev->streaming_users reached 0.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---

Comments

shuah April 1, 2019, 2:54 p.m. UTC | #1
On 4/1/19 5:18 AM, Hans Verkuil wrote:
> If the application was streaming from both videoX and vbiX, and streaming
> from videoX was stopped, then the vbi streaming also stopped.
> 
> The cause being that stop_streaming for video stopped the subdevs as well,
> instead of only doing that if dev->streaming_users reached 0.
> 
> au0828_stop_vbi_streaming was also wrong since it didn't stop the subdevs
> at all when dev->streaming_users reached 0.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
> diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
> index e17047b94b87..fb8e42c74653 100644
> --- a/drivers/media/usb/au0828/au0828-video.c
> +++ b/drivers/media/usb/au0828/au0828-video.c
> @@ -861,10 +861,11 @@ static void au0828_stop_streaming(struct vb2_queue *vq)
> 
>   	dprintk(1, "au0828_stop_streaming called %d\n", dev->streaming_users);
> 
> -	if (dev->streaming_users-- == 1)
> +	if (dev->streaming_users-- == 1) {
>   		au0828_uninit_isoc(dev);
> +		v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
> +	}
> 
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
>   	dev->vid_timeout_running = 0;
>   	del_timer_sync(&dev->vid_timeout);
> 
> @@ -893,8 +894,10 @@ void au0828_stop_vbi_streaming(struct vb2_queue *vq)
>   	dprintk(1, "au0828_stop_vbi_streaming called %d\n",
>   		dev->streaming_users);
> 
> -	if (dev->streaming_users-- == 1)
> +	if (dev->streaming_users-- == 1) {
>   		au0828_uninit_isoc(dev);
> +		v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
> +	}
> 
>   	spin_lock_irqsave(&dev->slock, flags);
>   	if (dev->isoc_ctl.vbi_buf != NULL) {
> 


Thanks. I will test the series with this patch and send v15

thanks again,
-- Shuah
shuah April 1, 2019, 10:01 p.m. UTC | #2
Hi Hans,

On 4/1/19 8:54 AM, shuah wrote:
> On 4/1/19 5:18 AM, Hans Verkuil wrote:
>> If the application was streaming from both videoX and vbiX, and streaming
>> from videoX was stopped, then the vbi streaming also stopped.
>>
>> The cause being that stop_streaming for video stopped the subdevs as 
>> well,
>> instead of only doing that if dev->streaming_users reached 0.
>>
>> au0828_stop_vbi_streaming was also wrong since it didn't stop the subdevs
>> at all when dev->streaming_users reached 0.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>> diff --git a/drivers/media/usb/au0828/au0828-video.c 
>> b/drivers/media/usb/au0828/au0828-video.c
>> index e17047b94b87..fb8e42c74653 100644
>> --- a/drivers/media/usb/au0828/au0828-video.c
>> +++ b/drivers/media/usb/au0828/au0828-video.c
>> @@ -861,10 +861,11 @@ static void au0828_stop_streaming(struct 
>> vb2_queue *vq)
>>
>>       dprintk(1, "au0828_stop_streaming called %d\n", 
>> dev->streaming_users);
>>
>> -    if (dev->streaming_users-- == 1)
>> +    if (dev->streaming_users-- == 1) {
>>           au0828_uninit_isoc(dev);
>> +        v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
>> +    }
>>
>> -    v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
>>       dev->vid_timeout_running = 0;
>>       del_timer_sync(&dev->vid_timeout);
>>
>> @@ -893,8 +894,10 @@ void au0828_stop_vbi_streaming(struct vb2_queue *vq)
>>       dprintk(1, "au0828_stop_vbi_streaming called %d\n",
>>           dev->streaming_users);
>>
>> -    if (dev->streaming_users-- == 1)
>> +    if (dev->streaming_users-- == 1) {
>>           au0828_uninit_isoc(dev);
>> +        v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
>> +    }
>>
>>       spin_lock_irqsave(&dev->slock, flags);
>>       if (dev->isoc_ctl.vbi_buf != NULL) {
>>
> 
> 
> Thanks. I will test the series with this patch and send v15
> 

I made VBI shareable and ran tests with your patch. If I were to start
VBI capture while VBI stream is active, the active VBI stream stops:

1. Start qv4l2 -V /dev/vbi0
     Start capture - Streaming active

2. Start qv4l2 -V /dev/vbi0
    Start capture - Streaming active
    Streaming won't start and the first stream stops.

I think there is still something that needs fixing in au0828.

If I were to start v4l2-ctl video streaming and then start
VBI streams - all 3 work correctly. I think

I am sending the patch series with audio, video, and VBI shareable
with this known issue. We will have to figure this out.

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index e17047b94b87..fb8e42c74653 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -861,10 +861,11 @@  static void au0828_stop_streaming(struct vb2_queue *vq)

 	dprintk(1, "au0828_stop_streaming called %d\n", dev->streaming_users);

-	if (dev->streaming_users-- == 1)
+	if (dev->streaming_users-- == 1) {
 		au0828_uninit_isoc(dev);
+		v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
+	}

-	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
 	dev->vid_timeout_running = 0;
 	del_timer_sync(&dev->vid_timeout);

@@ -893,8 +894,10 @@  void au0828_stop_vbi_streaming(struct vb2_queue *vq)
 	dprintk(1, "au0828_stop_vbi_streaming called %d\n",
 		dev->streaming_users);

-	if (dev->streaming_users-- == 1)
+	if (dev->streaming_users-- == 1) {
 		au0828_uninit_isoc(dev);
+		v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
+	}

 	spin_lock_irqsave(&dev->slock, flags);
 	if (dev->isoc_ctl.vbi_buf != NULL) {