Conversation
joaoantoniocardoso
force-pushed
the
support_all_raw_formats
branch
2 times, most recently
from
ede3fa3 to
6bf19ba
Compare
joaoantoniocardoso
force-pushed
the
support_all_raw_formats
branch
from
6bf19ba to
f842429
Compare
joaoantoniocardoso
force-pushed
the
support_all_raw_formats
branch
from
f842429 to
3148be5
Compare
| pub const KNOWN_RTP_RAW_FORMATS: [&str; 9] = [ | ||
| "RGB", "RGBA", "BGR", "BGRA", "AYUV", "UYVY", "I420", "Y41B", "UYVP", | ||
| ]; | ||
| pub const DEFAULT_RAW_FORMAT: &str = "I420"; |
There was a problem hiding this comment.
Did you look at https://docs.rs/gstreamer-video/latest/gstreamer_video/struct.VideoFormatInfo.html#method.name ?
Where the types are defined here:
https://docs.rs/gstreamer-video/latest/gstreamer_video/enum.VideoFormat.html
let info = VideoFormatInfo::from_format(crate::VideoFormat::RGB);
dbg!(info)'
| "H264" => return VideoEncodeType::H264, | ||
| "MJPG" => return VideoEncodeType::Mjpg, | ||
| // TODO: We can implement a [GstDeviceMonitor](https://gstreamer.freedesktop.org/documentation/gstreamer/gstdevicemonitor.html?gi-language=c) and then this manual mapping between v4l's and gst's fourcc will not be neccessary anymore. | ||
| // A list of possible v4l fourcc from the Linux docs: |
There was a problem hiding this comment.
I believe that my previous comment shows how to do it with gstreamer VideoFormatInfo
There was a problem hiding this comment.
| if let Ok(c_char_array) = CString::new(fourcc.clone()).map(|c_str| c_str.into_raw()) { | ||
| use gst_video::ffi::*; | ||
|
|
||
| let gst_video_format = unsafe { gst_video_format_from_string(c_char_array) }; |
There was a problem hiding this comment.
I believe that my previous comment would also simplify this code.
| VideoEncodeType::Unknown(fourcc) | ||
| } | ||
| } | ||
| impl<'de> Deserialize<'de> for VideoEncodeType { |
There was a problem hiding this comment.
please add a space between the implementations.
| VideoEncodeType::Unknown(fourcc) | ||
| } | ||
| } | ||
| impl<'de> Deserialize<'de> for VideoEncodeType { |
There was a problem hiding this comment.
Can you provide more information about why this implementation is necessary ? looking at the code this is not clear why.
There was a problem hiding this comment.
Because of the tricky nature of having two Enum fields both encapsulating Strings, plus to create a validation of what is accepted from GStreamer/V4l, I had to write a custom deserializer.
Like, if we define the Unknown(String) as untagged, everything different from the other options will be Unknown. If we then add another untagged Raw(String), then it goes to who is defined first, and we actually want to use custom logic to define if it is Raw or Unknown.
There was a problem hiding this comment.
So, it would be nice to have such explanation as a comment.
2 participants
This patch adds a
Raw(String), instead of defining each known Raw format by hand.There are three main fragilities for this approach:
fourcc(I defined them asKNOWN_RTP_RAW_FORMATS)fourccs from thev4lAPI (defined by the Linux Kernel) don't have the same name as thefourccs used by GStreamer, so we'd always need to manually create the map from one to the other, like v4l'sYUYVto GSt'sYUY2. An alternative is to use a GstDeviceMonitor to get that information instead of using the v4l API directly. (I wrote this issue to tackle that)Despite that, it seems to be working for those formats that were already working, and I can't test other formats because all cameras I have are either H264, MJPG, or YUYV.