v4l2: fix ALIGN(pix->width, 7); second commit re-does 545cb18 and should wait

[vrilutza]

Before anything else: the second commit here re-does part of 545cb18 (May 2020), where you deliberately went back to discrete because Skype could not handle a stepwise range — with the same step_width = 8, step_height = 1. I did not find that before opening this, and the first version of this description cited #52 as support when #52 is part of what led you to revert. Sorry for the noise.

The two commits now have very different standing.

Commit 1 — ALIGN(pix->width, 7). Plain bug, take it whenever.

ALIGN(x, a) expands to ((x) + (a - 1)) & ~(a - 1) and needs a power of two. With 7 the mask clears bits 1 and 2 but not bit 0, so the parity of the request survives and every odd width stays odd — and YUYV, two pixels in four bytes, cannot represent one.

The driver then contradicts itself, two commands apart on the same device:

$ v4l2-ctl --list-frameintervals=width=329,height=240,pixelformat=YUYV   -> EINVAL
$ v4l2-ctl --list-frameintervals=width=328,height=240,pixelformat=YUYV   -> 30 fps

S_FMT had just granted 329. enum_frameintervals() has rejected non-multiples of 8 since 6bbe371 (28 Nov 2015); the broken ALIGN() arrived in 230e57a two days later. Independent of commit 2 and of 545cb18.

Commit 2 — stepwise enumeration. Please do not take this yet.

What the revert cost. Since 545cb18 the driver advertises one size while adjust_format() accepts anything from 320x240 up to the sensor. #243 (2021) and #323 (2026) are both people who cannot get a lower resolution out of an application, and both were opened after it.

Why it might be different now. The Skype desktop client that broke in 2016 is gone; applications reach the camera through PipeWire or GStreamer.

Why I would still not merge it today. I measured it, and the 2020 problem has not gone away — it moved. On a released PipeWire a stepwise range makes GNOME Snapshot negotiate 320x240 instead of 1296x736, and take ~4100 ms to a continuous viewfinder instead of ~1500 ms. Same class of failure as Skype, different application. Numbers in the note below. Both are PipeWire bugs with fixes posted upstream, but until those land this would make the camera worse than it is.

Happy to split the two into separate PRs if that is easier to handle.

---

The series

Seven PRs on this driver, all on master (364b1c6), all found while debugging a camera problem on a MacBookPro14,1:

#328	two small cleanups: missing break, hardcoded buffer count
#329	control values discarded at every VIDIOC_STREAMON
#330	cmd.y1 never assigned in the crop command — one line
#331 (this one)	ALIGN(width, 7) aligns nothing; second commit re-does 545cb18 and should wait
#332	one firmware timeout leaves the driver with no usable buffers
#333	the camera writes in front of any buffer that is not page aligned
#334	the auto-exposure wait: 1000 ms → 200 ms

They can be taken in any order, with one exception: the second commit of #331 should not be taken yet — reasons in that PR.

(Until 23 August there was a second exception: #328 and #334 both carried the same mdelay → msleep commit, so whichever merged first made the other conflict. I have dropped it from #328; it lives in #334. The seven are now independent of each other.)

There is one more patch not sent yet: the channel-start crop returns the top left corner of the frame at any size below the sensor. It is measured and ready, and it sits on top of #330. I am holding it back rather than adding an eighth PR to the pile.

[vrilutza]

Worth knowing before this is merged, and it is not a reason not to merge it.

Reporting a real stepwise range is correct. But on a released PipeWire it has two user-visible
effects, both because a range reaches GStreamer unfixed and gst_caps_fixate() resolves a range to
its minimum. Measured with GNOME Snapshot, three runs each, the only difference being the
enumeration commit reverted on top of the rest:

	with this patch	reverted
negotiated format	320x240	1296x736
stream restarts while starting	3	1
time to a continuous viewfinder	~4100 ms	~1500 ms

The resolution is the obvious one. The startup cost is the surprising one: with a range in the caps,
a check in pipewiresrc that is meant to skip pointless renegotiations can never match, so every
reconfigure event tears the stream down and brings it back — and this camera spends about a second
in VIDIOC_STREAMON each time.

Both are PipeWire defects, not driver ones, and both already have fixes posted:
!2950 and
!2951. With them applied
this patch costs nothing.

So it is not wrong — it exposes two things already broken for any camera reporting a range. But as of
today, merging it makes this camera slower to open and lower resolution than before. If you would
rather wait for the PipeWire side, that is a fair call; I am flagging it so it is your choice rather
than a surprise.

(Edited 22 Aug: this branch now also carries the ALIGN(pix->width, 7) fix, which came over from
#329 after this was written. The measurements above do not concern it.)

[patjak]

I've cherry picked the alignment fix. Let's wait for userspace to be fixed before merging the other.