On Thu, May 11, 2023 at 05:29:14PM +0200, Hans Verkuil wrote:
> Hi Deb,
>
> When testing this I was wondering why the sequence counter didn't detect dropped
> frames (which happens when you start/stop a vbi stream while streaming video).
>
> On 02/05/2023 05:27, Deborah Brouwer wrote:
> > -static void bttv_field_count(struct bttv *btv)
> > -{
> > - int need_count = 0;
> > -
> > - if (btv->users)
> > - need_count++;
> > -
> > - if (need_count) {
> > - /* start field counter */
> > - btor(BT848_INT_VSYNC,BT848_INT_MASK);
> > - } else {
> > - /* stop field counter */
> > - btand(~BT848_INT_VSYNC,BT848_INT_MASK);
> > - btv->field_count = 0;
> > - }
> > -}
>
> This is the root cause: this function is used to turn on VSYNC interrupts
> and in the interrupt handler the field_count is incremented.
>
> In the vb1 version of this driver this field_count is passed on to vb1, which
> uses it to set the sequence counter to field_count / 2.
>
> By removing this function the VSYNC irq is never enabled and so field_count is
> always 0. So I think in bttv the seqnr field should be dropped and the field_count
> mechanism re-instated.
>
> Comparing the number of dropped frames when starting/stopping vbi it looks like
> in both cases (vb1 and vb2) one frame is dropped when starting vbi. But when
> stopping vbi no frames are dropped in the vb1 case, but 3 in the vb2 case.
Ok, I used btv->field_count >> 1 to set the sequence numbers and I added VSYNC to
the interrupt mask when start_streaming() is called. I can see the dropped video frames
in v4l2-ctl when vbi starts and stops (or vice versa), but I am not sure if this means
that we are actually dropping any valid, captured pixels.
The internal FIELD change signal (even/odd) that is picked up by VSYNC seems to run
independently from capturing. For example, out of curiosity, I added VSYNC to the
interrupt mask at probe, and it starts to increment field_count (and so the sequence
numbers) immediately and continuously even though capture isn't enabled.
So I am wondering if field_count is a totally reliable way to assign sequence numbers
to the buffers. For example, using field_count, the buffer sequence numbers start with
3 instead of 0 in both vb1 and vb2 (sometimes it starts at 2 in vb2).
Maybe the reason for this is that the field signal (even/odd) continues to be sent while
the captured pixels are being input into the chip's FIFO buffer. The FIFO buffer is
preparing the pixels for output by adding the timing/control
information but hasn't yet made them available to be picked up by the DMA controller for
output to memory. So, there is a disconnect, when streaming starts, between field_count
(which keeps incrementing) and the actual availability of pixels.
Maybe there is a similar disconnect happening when vbi starts or stops. Instead of actually
dropping pixels what we really have is just a delay in the RISC program that has to cycle
through its instructions. vb2 has made this a bit worse when streaming stops because we wait
for all the buffers to finish. I will investigate more if there is an easy way to fix this in vb2
without changing the RISC program.
>
> 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.
Interesting to know that this happened in your tests also.
Obviously the easy solution would be just to enable the vcr_hack by default.
It removes the last four scan lines and so prevents the risc program
counter from getting stuck at a buffer memory address (which definitely
causes dropped frames) instead of returning to the main risc program address.
>
> When I have more time I will dig into this a bit more.
>
> Regards,
>
> Hans
