On 11/09/2023 11:51, Hans Verkuil wrote: > Hi Deb, > > On 10/09/2023 04:33, Deborah Brouwer wrote: >> On Thu, Sep 7, 2023 at 8:23 AM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: >>> >>> Hi Deb, >>> >>> On 04/09/2023 13:51, Christian Zigotzky wrote: >>>> On 05 May 2023 at 09:20 am, Christian Zigotzky wrote: >>>>> On 05 May 2023 at 08:45 am, Hans Verkuil wrote: >>>>>> On 05/05/2023 08:25, Christian Zigotzky wrote: >>>>>>> On 02 May 2023 at 08:57 am, Hans Verkuil wrote: >>>>>>>> If v4l2-ctl fails, then try again >>>>>>>> after applying this series: >>>>>>>> >>>>>>>> https://patchwork.linuxtv.org/project/linux-media/cover/cover.1682995256.git.deborah.brouwer@xxxxxxxxxxxxx/ >>>>>>> Your patch series solved the issue. Thanks a lot! >>>>>> Nice, but somewhat unexpected :-) >>>>>> >>>>>> I'm a little bit unsure how to proceed: the 6.4 kernel will remove destructive overlay >>>>>> support, but it won't have this series yet, that's for 6.5. But that would make 6.4 >>>>>> unusable for you. >>>>>> >>>>>> I might have to revert the overlay removal, at least for bttv. >>>>>> >>>>>> Regards, >>>>>> >>>>>> Hans >>>>> Hans, >>>>> >>>>> You don't need to revert the overlay removal because your patch series work with the latest git kernel (6.4). >>>>> >>>>> Thanks for your help, >>>>> >>>>> Christian >>>> >>>> Hello Hans, >>>> >>>> I successfully used your patches for the kernel 6.5. Everything works without any problems with your patch series from May. >>>> >>>> Your patches have been added with the latest Media updates [1] for the kernel 6.6. >>>> >>>> The patches works but I have a green bar in the bottum of the window if I use the composite input. [2] >>>> >>>> I don't have this green bar with your May patch series. >>>> >>>> Could you please check your latest patches? >>>> >>>> Thanks, >>>> >>>> Christian >>>> >>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=307d59039fb26212a84a9aa6a134a7d2bdea34ca >>>> [2] https://i.ibb.co/D4K6j2c/Kernel-6-6-alpha2-Power-PC.png >>>> >>>> >>> >>> After some testing and debugging I found this change in the bttv vb2 conversion >>> patch: >>> >>> https://patchwork.linuxtv.org/project/linux-media/patch/d785cd8b0d8c19c05fcaa1536622e2fdd9f8ede3.1689379982.git.deborah.brouwer@xxxxxxxxxxxxx/ >>> >>>> diff --git a/drivers/media/pci/bt8xx/bttv-risc.c b/drivers/media/pci/bt8xx/bttv-risc.c >>>> index 3e0dac56de54..436baf6c8b08 100644 >>>> --- a/drivers/media/pci/bt8xx/bttv-risc.c >>>> +++ b/drivers/media/pci/bt8xx/bttv-risc.c >>>> @@ -67,8 +67,10 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc, >>>> /* scan lines */ >>>> sg = sglist; >>>> for (line = 0; line < store_lines; line++) { >>>> - if ((btv->opt_vcr_hack) && >>>> - (line >= (store_lines - VCR_HACK_LINES))) >>>> + if ((line >= (store_lines - VCR_HACK_LINES)) && >>>> + (btv->opt_vcr_hack || >>>> + (V4L2_FIELD_HAS_BOTH(btv->field) || >>>> + btv->field == V4L2_FIELD_ALTERNATE))) >>>> continue; >>>> while (offset && offset >= sg_dma_len(sg)) { >>>> offset -= sg_dma_len(sg); >>> >>> It is this change that causes the issue: basically the condition >>> (V4L2_FIELD_HAS_BOTH(btv->field) || btv->field == V4L2_FIELD_ALTERNATE) >>> is almost always true (it is only false for FIELD_TOP/BOTTOM), so it is >>> as if vcr_hack is always turned on. >>> >>> It looks to me like some debug code accidentally ended up in this patch. >>> Reverting this change makes everything look good again (Christian, it would >>> be great if you can confirm that this also fixes the issue for you!). >>> >>> Deb, can you remember anything about this change? >> >> This is not debug code. I made the change to fix the latency and >> frame drop issues >> that were otherwise occurring with vb2; as I said in the cover letter to v3: >> >> "- remove the last four lines in interlaced, >> sequential top/bottom, and alternating buffers >> to prevent long latency and frame drops >> (this is instead of just enabling the existing >> VCR hack by default);" >> https://lore.kernel.org/linux-media/cover.1689379982.git.deborah.brouwer@xxxxxxxxxxxxx/ >> >> However, if your testing shows that it isn't needed, then it would be >> fine to remove this >> code and just let the user enable the "vcr hack" as needed. This was >> my original approach >> in v2, but I thought you had said at the time that you were seeing >> massive framedrops in v2? > > That's not quite what I said: > > "Another thing I discovered is that for PAL the vcr_hack control has to be enabled, > otherwise the video is full of glitches. This was present before your series, and > happens even with a video signal from a proper PAL video generator, so this is really > strange. I can't remember that I needed this in the past, but it has been years > since I last tested it. > > PAL capture is fine for Top/Bottom/Alternate settings, it only fails for Interlaced > and Sequential Top/Bottom capture modes." > > This issue was present before your series, so this is not something that should be > changed in a vb2 conversion, since this has nothing to do with it. > > In addition, it is a PAL only issue, and this change would turn on vcr_hack for NTSC > as well, and also for FIELD_ALTERNATE, even though that is not affected by it. > > Interestingly, since I was wondering why Christian didn't see it even though he is > in a PAL country, I tested with tvtime and that displays it fine, but qv4l2/qvidcap > show the glitches. This suggests that it is a problem in those utils, and not in the > driver. I'll dig deeper into this. There is definitely something going on with qv4l2/qvidcap, although I have still not been able to figure it out. Those two applications have very similar openGL shader code, so I suspect it is related to that, but what exactly is going wrong there is not clear to me. Regards, Hans > > In the meantime, I'll prepare a patch reverting this change for v6.6. > > Regards, > > Hans > >> >> I didn't notice this green line before because I was testing in qv4l2 >> with the default >> Pixel Format : 'BGR3' (24-bit BGR 8-8-8) whereas tvtime is using >> YUYV' (YUYV 4:2:2) >> >> One fix that worked for me was to adjust the "OverScan" configuration in tvtime >> so that it is at least 3.5. The /etc/tvtime/tvtime.xml configuration >> file recommends >> even higher at 8.0. Christian, please try adjusting the overscan >> value to see if >> that is a possible solution as well. >> >> Thanks, >> Deb >> >>> >>> Regards, >>> >>> Hans >>> >>> >>> >
