src: Get video formats using Device Monitor#281
Conversation
3639f6f to
e25d761
Compare
ab854c7 to
7bb361a
Compare
|
@joaoantoniocardoso can you rebase this PR ? |
7bb361a to
f85a476
Compare
|
@patrickelectric oops, accidentally clicked to merge before review; Can you review it and I make a following PR to adjust any discrepancies? |
patrickelectric
left a comment
There was a problem hiding this comment.
I had no time to finish the review today. Will do the final commit tomorrow.
| info!("GST Device Providers: {providers:#?}"); | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
just for curiosity, is there a reason for not having this in the default call of Manager ?
| }; | ||
|
|
||
| device.properties().iter().any(|s| { | ||
| let Ok(api) = s.get::<String>("device.api") else { |
There was a problem hiding this comment.
It would be nice to have a comment for this string:
# device.api is an inner string defined by gst for which api this device is accessed with: Ex: alsa, v4l..
It would be even better to have such strings as "device.path" to be defined as a global private property that explains that is a GST property string for this field
| let device = device_weak.upgrade()?; | ||
| let name = device.display_name().to_string(); | ||
| let properties = device.properties()?; | ||
| let device_path = properties.get::<String>("device.path").ok()?; | ||
| let bus = properties.get::<String>("v4l2.device.bus_info").ok()?; |
There was a problem hiding this comment.
isn't it better to have abstraction for it under gst_device_monitor as the function device_caps ?
| Yuyv, | ||
| } | ||
|
|
||
| #[derive(Apiv2Schema, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize)] |
There was a problem hiding this comment.
This commit is unnecessary moving this structure.
There was a problem hiding this comment.
I'm slowly moving things to get a top-to-bottom declaration, but if I added another commit, we'd have multiple commits touching the same lines.
| for candidate in candidates.iter() { | ||
| let VideoSourceType::Local(camera) = candidate else { | ||
| continue; | ||
| }; | ||
| if camera.name != source.name { | ||
| continue; | ||
| } | ||
| formats.insert( | ||
| candidate.inner().source_string().to_string(), | ||
| candidate.formats().await, | ||
| ); | ||
| } |
There was a problem hiding this comment.
| for candidate in candidates.iter() { | |
| let VideoSourceType::Local(camera) = candidate else { | |
| continue; | |
| }; | |
| if camera.name != source.name { | |
| continue; | |
| } | |
| formats.insert( | |
| candidate.inner().source_string().to_string(), | |
| candidate.formats().await, | |
| ); | |
| } | |
| for candidate in candidates.iter().filter(|candidate| { | |
| matches!(candidate, VideoSourceType::Local(camera) if camera.name == source.name) | |
| }) { | |
| formats.insert( | |
| candidate.inner().source_string().to_string(), | |
| candidate.formats().await, | |
| ); | |
| } |
|
|
||
| fn get_device_formats_using_gstreamer( | ||
| device_path: &str, | ||
| _typ: &VideoSourceLocalType, |
There was a problem hiding this comment.
why is this necessary ?
There was a problem hiding this comment.
The main idea is to use the DeviceMonitor's Device to interface the "local" kind of sources, and the previous approach to get formats was specific to v4l, so it wouldn't work for libcamerasrc or other sources.
Merge after #607
update 13/05/26:
This PR is the first step towards supporting libcamera. In this step, we move from the v4l crate to give us the stream formats to get them from the GStreamer DeviceMonitor.
The further steps are already planned, and libcamera will be functional in the next PR.
With these changes, the v4l crate is only used for the controls API (which is fine).
update 09/29/25:
This seems very promising -- so far, I have already replaced the v4l formats and the v4l device discovery.
I'm thinking we should refactor the source/pipeline part of the application to fully make use of this Device Monitor:
Helps #372, #273, #139, #79