Refactor emulated_camera to use a shared video_capture_device library#2653
Refactor emulated_camera to use a shared video_capture_device library#2653bridadan wants to merge 3 commits into
Conversation
ser-io
left a comment
ser-io
left a comment
There was a problem hiding this comment.
Thanks for this needed refactoring.
Can you split the PR into two commits (parts)
- Refactor code that's clearly duplicated among the two devices implemention without
CaptureDeviceFormatabstraction. - Introduce the new CaptureDeviceFormat abstraction and the refactored code around it.
I'd like to analyze the new abstraction of a commit of its own. Thanks.
bridadan
force-pushed
the
dedup_virtio_media_code
branch
from
774f514 to
3ba52ec
Compare
|
@ser-io Here's an attempt at splitting up the changes. The |
| } | ||
| } | ||
|
|
||
| pub fn enum_framesizes( |
There was a problem hiding this comment.
This helper function is not extensible. If I were to add a new frame size to my device implementation, I cannot reuse this function.
Let's keep this responsibility on the device implementation side.
|
|
||
|
|
||
|
|
||
| pub fn default_fmtdesc(queue: QueueType, pixelformat: u32) -> v4l2_fmtdesc { |
There was a problem hiding this comment.
No need to introduce this new helper as it just return a struct. It's more clear to return the struct directly on the device implementation.
| }) | ||
| } | ||
|
|
||
| pub fn enum_frameintervals( |
| @@ -0,0 +1,10 @@ | |||
| [package] | |||
| name = "video_capture_device" | |||
There was a problem hiding this comment.
This should be a new module pub mod devices in lib.rs
The directory "vhu_media" is the library code containing the shared code to be used by the binaries. Eventually we need to split the "lib.rs" file into different files. Feel free to do this if you want in this PR, however it's not a requirement for this PR.
| }) | ||
| } | ||
|
|
||
| pub fn configure_planes(v4l2_buffer: &mut V4l2Buffer, offset: u32, buffer_size: u32) { |
There was a problem hiding this comment.
We can be specific with the function name for readability in this case: set_plane_offset_and_length.
Extract shared code from `emulated_camera_mplane` and `emulated_camera_splane` into a new `devices` module of `vhu_media`. This version extracts only obviously duplicated code (structs, enums, and helper functions). Key changes: - Created `devices` module in `vhu_media` library containing: - `Buffer` and `BufferState` implementations. - Refactored `emulated_camera_mplane` and `emulated_camera_splane` to: - Depend on the new `devices` module. - Use the shared `Buffer` and `BufferState`. - Updated Cargo.toml and BUILD.bazel files to reflect new dependencies. TAG=agy CONV=b4f9ffcf-c393-4dbc-b8fb-5c38416291c2 Bug: 519646531 Test: cd base/cvd && bazel build cuttlefish/package:cvd
…ptureDeviceFormat trait
Extract `EmulatedCamera` and `EmulatedCameraSession` structs and their
`VirtioMediaDevice` and `VirtioMediaIoctlHandler` implementations into the shared
`vhu_media` library to eliminate duplication.
Introduce the `CaptureDeviceFormat` trait in the shared library to abstract
format-specific differences between `emulated_camera_mplane` and
`emulated_camera_splane`.
Key changes:
- Created `CaptureDeviceFormat` trait in `vhu_media`.
- Moved `Buffer`, `BufferState`, `EmulatedCameraSession`, and `EmulatedCamera`
to `vhu_media`, making the camera and session generic over
`F: CaptureDeviceFormat`.
- Moved and unified `VirtioMediaDevice` and `VirtioMediaIoctlHandler`
implementations to `vhu_media`.
- Refactored `emulated_camera_mplane` and `emulated_camera_splane` to:
- Implement `CaptureDeviceFormat` for `MplaneFormat` and `SplaneFormat` respectively.
- Implement `default_fmt` and `write_pattern` directly within the trait
impl blocks.
- Instantiate the generic `EmulatedCamera` in `main.rs`.
TAG=agy
CONV=b4f9ffcf-c393-4dbc-b8fb-5c38416291c2
Bug: 519646531
Test: cd base/cvd && bazel build cuttlefish/package:cvd
# Repeat with v4l2_emulated_camera_mplane
cvd create -media=type=v4l2_emulated_camera_splane
# Ensure pixel format changes with each type
v4l2-ctl -d1 --all
# Ensure it passes with no failures
v4l2-compliance -d1 -s
bridadan
force-pushed
the
dedup_virtio_media_code
branch
from
3ba52ec to
33297c9
Compare
Bug: 519646531
Test: cd base/cvd && bazel build cuttlefish/package:cvd
# Repeat with v4l2_emulated_camera_mplane
cvd create -media=type=v4l2_emulated_camera_splane
# Ensure pixel format changes with each type
v4l2-ctl -d1 --all
# Ensure it passes with no failures
v4l2-compliance -d1 -s
bridadan
force-pushed
the
dedup_virtio_media_code
branch
from
33297c9 to
7c6c405
Compare
2 participants
Extract shared code from
emulated_camera_mplaneandemulated_camera_splaneinto a new shared libraryvideo_capture_device. This removes a large amount of copy-pasted code between the two emulated camera implementations.Key changes:
video_capture_devicelibrary containing:CaptureDeviceFormattrait defining format-specific constants and methods.EmulatedCameraandEmulatedCameraSessionstructs implementingVirtioMediaDeviceandVirtioMediaIoctlHandler.BufferandBufferStateimplementations.configure_planesin the trait, as it was identical for both devices.emulated_camera_mplaneandemulated_camera_splaneto:video_capture_devicelibrary.CaptureDeviceFormatfor their respective format structs (MplaneFormatandSplaneFormat).EmulatedCamerato the generic one.TAG=agy
CONV=b4f9ffcf-c393-4dbc-b8fb-5c38416291c2
Bug: 519646531
Test: build and launch with both device types, ensure v4l2-compliance
passes in the guest