lkml.org 
[lkml]   [2019]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v14 5/6] au0828: fix enable and disable source audio and video inconsistencies
From
Date
On 4/1/19 5:14 AM, Hans Verkuil wrote:
> Hi Shuah,
>
> On 3/30/19 2:54 AM, Shuah Khan wrote:
>> Enable and disable source interfaces aren't consistent in enforcing
>> how video and audio share the tuner resource.
>>
>> Fix these issues to enforce the following rules and allow
>> sharing between audio and video applications.
>>
>> - When DVB is streaming, audio/video/vbi/s-video/composite
>> should find the resource busy. DVB holds the tuner in
>> exclusive mode.
>> - When video is streaming, audio can share the tuner and vice versa.
>> - v4l2 allows multiple applications to open video device.
>> - Video applications call enable source multiple times during their
>> run-time. Resource should stay locked until the last application
>> releases it.
>> - A shared resource should stay in shared state and locked when it is
>> in use by audio and video. More than one video application is allowed
>> to use the tuner as long as video streaming protocol allows such usage.
>> Resource is released when the last video/audio application releases it.
>> - S-Video and Composite hold the resource in exclusive mode.
>> - VBI allows more than vbi applications sharing and will not share
>> with another type. When resource is locked by VBI and in use by
>> multiple VBI applications, it should stay locked until the last
>> application disables it.
>
> This isn't correct: only one application at most can stream VBI, but
> VBI can be shared with video and audio streaming.
>
> All three come from the same source (analog TV), so as long as one of
> these is streaming the analog tuner resource is in use.
>
> With this patch it seems that streaming VBI blocks streaming audio/video
> and vice versa. That's wrong.

Right. The reason I went this route is that VBI streaming stops even
when another VBI starts to stream. I am debugging this. Looks like
you have a fix for this from reading below.

>
> Otherwise everything looks OK, except for a typo below.
>

Will fix the typos and also I noticed, I forgot to catch updating
Copyright in one of the files.

>>
>> Signed-off-by: Shuah Khan <shuah@kernel.org>
>> ---
>> drivers/media/usb/au0828/au0828-core.c | 182 +++++++++++++++++++------
>> drivers/media/usb/au0828/au0828.h | 5 +-
>> 2 files changed, 148 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
>> index 4f8ba6f64d3c..b8bcab2fa1dc 100644
>> --- a/drivers/media/usb/au0828/au0828-core.c
>> +++ b/drivers/media/usb/au0828/au0828-core.c
>> @@ -272,6 +272,27 @@ static void au0828_media_graph_notify(struct media_entity *new,
>> }
>> }
>>
>> +static bool au0828_is_link_sharable(struct media_entity *owner,
>> + struct media_entity *entity)
>> +{
>> + bool sharable = false;
>
> typo: sharable -> shareable
>
> Also in the function name: do a search-and-replace for this.

Will do.

>
>> +
>> + /* Tuner link can be shared by audio and video */
>
> add: and VBI
>
> I think the core issue is here since VBI should be included with
> IO_V4L and AUDIO_CAPTURE.
>
>> + switch (owner->function) {
>> + case MEDIA_ENT_F_IO_V4L:
>> + case MEDIA_ENT_F_AUDIO_CAPTURE:
>> + if (entity->function == MEDIA_ENT_F_IO_V4L ||
>> + entity->function == MEDIA_ENT_F_AUDIO_CAPTURE)
>> + sharable = true;
>> + break;
>> + case MEDIA_ENT_F_DTV_DEMOD:
>> + case MEDIA_ENT_F_IO_VBI:
>> + default:
>> + break;
>> + }
>> + return sharable;
>
> I hacked this function so VBI is included with IO_V4L/AUDIO_CAPTURE
> and now it works.
>

Yes. Y have been debugging with VBI included in the sharing, however
running into the below problem. Do you have the patch I can use to
test with this series?

> There is one remaining issue (but that was always a problem with au0828):
> when streaming is in progress from both video and vbi, and then you stop
> the video streaming, then vbi stops as well.
>
> au0828_stop_streaming() calls v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
> which it should only do when the last user disappears. I'll post a patch
> for au0828 to fix this.
>

Thanks for finding this problem. I narrowed it to close and didn't get
a chance to play with it.

thanks,
-- Shuah

\
 
 \ /
  Last update: 2019-04-01 16:54    [W:0.101 / U:0.328 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site