Re: saa7146: please test the vb2 conversion!

[Stefan Herdler <herdler@nurfuerspam.de>]

On 14/04/23 10:36 Hans Verkuil wrote:
> On 14/04/2023 02:15, Stefan Herdler wrote:
>> 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.
>
> Good news! I'll prepare a pull request later today.
>
>>
>>>>
>>>> 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.
>
> Is this something that also happens with the old (vb1) version of
> the driver? I'm not sure if this is related to my changes at all.

I am pretty sure, that it was not related to the changes.
It happened with the old (vb1) driver of the stock kernel too.

And it happened only at this single day.
I have tried at least 50 times since then and it didn't happen again.

I'm sorry for the confusion, there must have been something else
wrong that day. But I still don't really know what.



Regards

Stefan


>
> I don't see how qv4l2 can cause this to happen since trying to change
> the format while tvtime is running will just return EBUSY and not do
> anything else.
>
> Regards,
>
> 	Hans
>
>>
>>
>>
>>
>> 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
>>>>>>
>>>>> [...]
>>>>
>>>
>