simple_device: Make fully comply with v4l2-compliance#26
Conversation
This commit introduces several foundational fixes and missing IOCTL implementations to allow the simple capture device to pass strict v4l2-compliance checks and properly support multi-planar (MPLANE) formats. Specifically: - Multi-planar mmap: Fixed mmap to correctly iterate through v4l2_planes and match the requested mem_offset, rather than assuming single-planar buffer offsets. - Plane bytesused: Updated Buffer state transitions to correctly reset and populate bytesused across all three YU12 planes. - Format capabilities: Implemented enum_framesizes and enum_frameintervals to explicitly advertise 1280x720 at 30fps. - Streaming Parameters: Implemented g_parm and s_parm to support framerate negotiation. - Queue Support: Expanded reqbufs, querybuf, qbuf, dqbuf, streamon, and streamoff to formally accept and configure both VideoCapture and VideoCaptureMplane queue types. Test: v4l2-compliance -d1 Signed-off-by: Changyeon Jo <changyeon@google.com>
|
I thought simple_device is already fully passing v4l2-compliance. |
| const PIXELFORMAT: u32 = PixelFormat::from_fourcc(b"YU12").to_u32(); | ||
| const WIDTH: u32 = 640; | ||
| const HEIGHT: u32 = 480; | ||
| const FRAME_RATE: u32 = 30; |
There was a problem hiding this comment.
Why remove FRAME_RATE? I saw it's still used everywhere.
| tv_sec: (sequence + 1) as bindings::__time_t / 1000, | ||
| tv_usec: (sequence + 1) as bindings::__time_t % 1000, | ||
| tv_sec: (sequence + 1) as bindings::time_t / 1000, | ||
| tv_usec: (sequence + 1) as bindings::time_t % 1000, |
There was a problem hiding this comment.
As tv_sec is (sequence + 1) / 1000, tv_usec should be (sequence + 1) % 1000 * 1000. Can we fix this bug?
| Ok(default_fmtdesc(queue)) | ||
| } | ||
|
|
||
| fn enum_framesizes( |
There was a problem hiding this comment.
This IOCTL is already supported. We don't need to move around.
| }) | ||
| } | ||
|
|
||
| fn enum_frameintervals( |
There was a problem hiding this comment.
This IOCTL is already supported. We don't need to move around.
| unsafe { | ||
| parm.parm.capture.capability = bindings::V4L2_CAP_TIMEPERFRAME; | ||
| parm.parm.capture.timeperframe.numerator = 1; | ||
| parm.parm.capture.timeperframe.denominator = 30; |
There was a problem hiding this comment.
Can we just use FRAME_RATE?
| capabilities: (BufferCapabilities::SUPPORTS_MMAP | ||
| | BufferCapabilities::SUPPORTS_ORPHANED_BUFS) | ||
| .bits(), | ||
| // This device does not support V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS, |
There was a problem hiding this comment.
I guess it's still worth keeping this documentation?
| @@ -281,9 +315,8 @@ where | |||
| } | |||
|
|
|||
| const PIXELFORMAT: u32 = PixelFormat::from_fourcc(b"YU12").to_u32(); | |||
There was a problem hiding this comment.
Should we just change to "YM12" for VideoCaptureMplane only support?
I'm wondering if we need to support both VideoCaptureMplane and VideoCapture on the device side.
| index: u32, | ||
| ) -> IoctlResult<v4l2r::ioctl::V4l2Buffer> { | ||
| if queue != QueueType::VideoCaptureMplane { | ||
| if queue != QueueType::VideoCaptureMplane && queue != QueueType::VideoCapture { |
There was a problem hiding this comment.
If we want to support both QueueType::VideoCaptureMplane and QueueType::VideoCapture, we should cover all IOCTLs (e.g. enum_fmt, g_fmt, ...etc).
3 participants
This commit introduces several foundational fixes and missing IOCTL implementations to allow the simple capture device to pass strict v4l2-compliance checks and properly support multi-planar (MPLANE) formats.
Specifically:
Test: v4l2-compliance -d1