Skip to content

Report the frame sizes the driver actually accepts (merge after #329 and #330) - #331

Open
vrilutza wants to merge 1 commit into
patjak:masterfrom
vrilutza:report-frame-sizes
Open

Report the frame sizes the driver actually accepts (merge after #329 and #330)#331
vrilutza wants to merge 1 commit into
patjak:masterfrom
vrilutza:report-frame-sizes

Conversation

@vrilutza

Copy link
Copy Markdown

Please merge this one last: after #329 and #330. It applies cleanly to master on its own, but it is only correct once those two are in — the reasons are below and in the commit message. This is the fourth and last piece of the same story, sent now so the whole set is in front of you rather than trickling in.

VIDIOC_ENUM_FRAMESIZES advertises a single discrete size while fthd_v4l2_adjust_format() clamps to FTHD_MIN_WIDTH/HEIGHT at the bottom and to the detected sensor size at the top, with the scaler covering everything in between. VIDIOC_ENUM_FRAMEINTERVALS already accepts any width that is a multiple of eight up to the maximum, so the enumeration is the only place claiming the device does one size and nothing else.

Applications that pick a resolution from the enumeration therefore never offer anything below the sensor's native size, even though the hardware scales down. That is what #323 describes — "the driver doesn't expose the camera in ways that an app can adjust resolution" — and what #243 is looking at when it prints Size: Discrete 1280x720. In #52 the reporter had to hand-edit Skype's shared.xml to force 640x480 and then 320x240 before the camera would work at all, and #36 is someone asking how to force a lower resolution; neither needed a driver change to capture at the smaller sizes, only a way to find out they exist.

This reports a stepwise range matching what adjust_format() does, keeping the sensor detection from 98b55fd as the upper bound: FTHD_MIN_WIDTH to the sensor width in steps of 8, FTHD_MIN_HEIGHT to the sensor height in steps of 1. The horizontal step of 8 is the constraint enum_frameintervals() has always enforced; nothing enforces one vertically, and heights of 241, 245 and 481 were verified to capture correctly, so the vertical step is 1.

Measured

MacBookPro14,1, where CISP_CMD_CH_CAMERA_CONFIG_GET reports a 1296x736 sensor:

[0]: 'YUYV' (YUYV 4:2:2)
        Size: Stepwise 320x240 - 1296x736 with step 8/1

and v4l2-compliance 1.32.0 goes from test Scaling: FAIL to test Scaling: OK.

Why it goes last

Taken in the order #329#330 → this, each step is an improvement on its own.

VIDIOC_ENUM_FRAMESIZES advertises a single discrete size while
fthd_v4l2_adjust_format() clamps to FTHD_MIN_WIDTH/HEIGHT at the bottom and to
the detected sensor size at the top, with the scaler covering everything
between. VIDIOC_ENUM_FRAMEINTERVALS already accepts any width that is a
multiple of eight up to the maximum, so the enumeration is the only place
claiming the device does one size and nothing else.

Applications that pick a resolution from the enumeration therefore never offer
anything below the sensor's native size, even though the hardware scales down.
In issue patjak#52 the reporter had to hand-edit Skype's shared.xml to force 640x480
and then 320x240 before the camera would work at all, and issue patjak#243 is a user
asking why the enumeration shows one size; neither needed a driver change to
capture at the smaller sizes, only a way to find out they exist.

Report a stepwise range matching what adjust_format() does, keeping the sensor
detection from 98b55fd as the upper bound: FTHD_MIN_WIDTH to the sensor width
in steps of 8, FTHD_MIN_HEIGHT to the sensor height in steps of 1.

The horizontal step of 8 is the constraint enum_frameintervals() has always
enforced. Nothing enforces one vertically, and heights of 241, 245 and 481 were
verified to capture correctly, so the vertical step is 1.

Tested on a MacBookPro14,1, where CISP_CMD_CH_CAMERA_CONFIG_GET reports a
1296x736 sensor, so the upper bound comes from the detection rather than from
FTHD_MAX_WIDTH/HEIGHT:

	[0]: 'YUYV' (YUYV 4:2:2)
		Size: Stepwise 320x240 - 1296x736 with step 8/1

and it takes v4l2-compliance 1.32.0 from "test Scaling: FAIL" to
"test Scaling: OK".

Depends on "v4l2: align the width to 8, not to 7" (patjak#329); without it the driver
would advertise a step of 8 that adjust_format() does not honour.

It should also not go in ahead of "isp: crop a centred window with the aspect
ratio of the output" (patjak#330). Until that one is in, fthd_start_channel() asks the ISP
for a crop window the size of the output placed at the sensor origin, so every
size below the sensor comes back as the top left corner of the frame.
Advertising the range first would hand applications exactly the resolutions that
are framed wrong.

Signed-off-by: Viorel Cernateanu <vrilutza@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant