Re: saa7146: please test the vb2 conversion!

[Stefan Herdler <herdler@nurfuerspam.de>]

On 12/04/23 13:16 Hans Verkuil wrote:
> On 12/04/2023 12:11, Hans Verkuil wrote:
>> On 10/04/2023 00:36, Stefan Herdler wrote:
>>> On 07/04/23 09:04, Hans Verkuil wrote:
>>>> On 07/04/2023 00:43, Stefan Herdler wrote:
>>> [...]
>>>>>
>>>>> VBI output is used to switch the aspect-ratio via WSS.
>>>>> this should be supported by any av7110 card.
>>>>>
>>>>> The software is run a daemon or plugin, so the userspace-facing change
>>>>> shouldn't matter.
>>>>>
>>>>> I'll test this as soon as possible.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> I've done only basic testing so far, but unfortunately it already failed.
>>>>>
>>>>> The test:
>>>>> Switch to a channel[*] and view the decoded video with tvtime.
>>>>>
>>>>> The resulting picture is corrupted.
>>>>> Almost green with some pink traces at the outlines.
>>>>>
>>>>> It reminds me to YCbCr component-yideo on a RGB-input.
>>>>> Maybe the input-format of saa7146 not set correctly?
>>>>>
>>>>> The OSD is equally affected, but the card seems to run stable.
>>>>
>>>> That's weird. When you are in this state, can you run
>>>> 'v4l2-ctl -V -d /dev/videoX' for the video device that tvtime
>>>> is using? I'll try to test it with tvtime as well next week.
>>>> I have done my tests using qvidcap and qv4l2, and that looked fine.
>>>
>>> I've done some more testing and the result is somehow confusing to me.
>>>
>>> At first I tried qv4l and it shows correct videos with any driver.
>>> And with any pixel format setting I tried.
>>>
>>>
>>> After boot /dev/video0 (there is only this device) starts always with
>>> this settings:
>>> Format Video Capture:
>>>         Width/Height      : 384/288
>>>         Pixel Format      : 'BGR3' (24-bit BGR 8-8-8)
>>>         Field             : Interlaced
>>>         Bytes per Line    : 1152
>>>         Size Image        : 331776
>>>         Colorspace        : SMPTE 170M
>>>         Transfer Function : Default (maps to Rec. 709)
>>>         YCbCr/HSV Encoding: Default (maps to ITU-R 601)
>>>         Quantization      : Default (maps to Full Range)
>>>         Flags             :
>>>
>>>
>>> On the working "old" driver tvtime switches to the following settings:
>>> Format Video Capture:
>>>         Width/Height      : 720/576
>>>         Pixel Format      : 'UYVY' (UYVY 4:2:2)
>>>         Field             : Interlaced
>>>         Bytes per Line    : 1440
>>>         Size Image        : 829440
>>>         Colorspace        : SMPTE 170M
>>>         Transfer Function : Default (maps to Rec. 709)
>>>         YCbCr/HSV Encoding: Default (maps to ITU-R 601)
>>>         Quantization      : Default (maps to Limited Range)
>>>         Flags             :
>>> It seems tvtime needs this 'UYVY' pixel format to work.
>>>
>>>
>>> On the "new" driver, with patches [1], tvtime switches to:
>>> Format Video Capture:
>>>         Width/Height      : 720/576
>>>         Pixel Format      : 'BGR3' (24-bit BGR 8-8-8)
>>>         Field             : Interlaced
>>>         Bytes per Line    : 2160
>>>         Size Image        : 1244160
>>>         Colorspace        : SMPTE 170M
>>>         Transfer Function : Default (maps to Rec. 709)
>>>         YCbCr/HSV Encoding: Default (maps to ITU-R 601)
>>>         Quantization      : Default (maps to Full Range)
>>>         Flags             :
>>> And now it is getting weird:
>>> I can switch to the correct 'UYVY' settings using qv4l.
>>> But tvtime always switches back to 'BGR3'.
>>
>> The cause is "[PATCH 10/17] media: common: saa7146: fall back to V4L2_PIX_FMT_BGR24".
>>
>> Can you drop that patch and test again?

Without this patch tvtime is working again.

>>
>> It's really a tvtime bug since drivers are allowed to either reject an unsupported
>> pixelformat (the old behavior) or replace it with a supported pixelformat (the
>> new behavior). And tvtime only supports the old behavior.
>
> FYI: I posted a patch fixing tvtime:
>
> https://patchwork.linuxtv.org/project/linux-media/patch/a5dff340-ab8a-46e0-1f0c-25ceaf9fe5ca@xs4all.nl/
>
> That said, I do plan to drop patch 10/17 from the saa7146 series, since it is better
> to keep the old behavior.
>
> Once I get the green light from you, I will make a pull request for this vb2 conversion.
>
> Regards,
>
> 	Hans
>
>>
>>>
>>> Using qv4l while tvtime is running doesn't work and sometimes
>>> causes freezing of both programs (on all drivers).
>>
>> Are you just starting qv4l2 when tvtime is running? Or trying to stream?
>> Do you see messages in the kernel log?
Just starting qv4l2 and trying to switch the pixelformat.
I haven't seen any related messages appearing while playing with qv4l2.
>>
>> I couldn't reproduce this. Since tvtime is streaming, qv4l2 shouldn't be able to
>> do anything since all attempts to change something should result in EBUSY.
Normally with qv4l2 it just happens nothing.
v4l2-ctl tells something like "device is busy".

I couldn't reproduce the freeze anymore today.
It was never really reproduce able, it only happened twice and
with different drivers. And it was always unexpected.

Today I found messages about an ARM-Crash on that particular day,
which I haven't seen before. They must have been come before or
after the tests.
The driver should recover the decoder, but sometimes it takes
a while until the it notices a crash.
So probably I haven't had connected the provisional cable to my
desktop properly that day, which caused a bad signal. That caused
an ARM-Crash, which caused the driver to freeze for a while.
And I killed applications to early.
At least that is the best explanation I have.




The vbi output is operational too.
I have verified, that the WSS bits change at the analog output.
The old tools are still working with the modified driver.


Regards

Stefan


>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>>
>>> I have also build a new driver just without the patches [2].
>>> It shows the "old" correct behavior.
>>> So I think, the cause of the change must be somewhere in the
>>> patches.
>>>
>>>
>>>
>>> Btw.:
>>> I also tried to open the video device with the usual
>>> media-players, but I had no luck so far (with any driver).
>>>
>>>
>>> Regards
>>>
>>> Stefan
>>>
>>>
>>> [1] git checkout -B saa7146-clean 837736a79a76c9becddf0caf905b27c144a64030
>>> [2] git checkout -B saa7146-clean 2653fad0d8a9625667e9a78133ea9e1245b7c40c
>>>
>>>>
>>>> Regards,
>>>>
>>>> 	Hans
>>>>
>>> [...]
>>
>