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. 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 >> >> >>
