-
Notifications
You must be signed in to change notification settings - Fork 230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
libav changes #634
libav changes #634
Conversation
@davidplowman can you cast your eyes on this please? Hopefully should give parity in the command line args between PI 4 and 5 for rpicam-vid. |
On Pi5, do not allow users to use the h264_v4l2m2m codec, as this will not work. Instead, revert to "libx264" as a default codec. Signed-off-by: Naushir Patuck <[email protected]>
else if (options->libav_format.empty()) | ||
format = nullptr; | ||
const std::string tcp { "tcp://" }; | ||
const std::string udp { "udp://" }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming we don't care about case here!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is case sensitive in libav.
encoder/libav_encoder.cpp
Outdated
{ | ||
if (options->output.empty() || | ||
options->output.find(tcp.c_str(), 0, tcp.length()) != std::string::npos || | ||
options->output.find(udp.c_str(), 0, udp.length()) != std::string::npos) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is slightly awkward to read, I find. Are we using C++20? Apparently that has a starts_with
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately we are using C++17, but starts_with is exactly what I want :(
if (options->listen && output_file_.find(tcp.c_str(), 0, tcp.length()) != std::string::npos) | ||
{ | ||
const std::string listen { "?listen=1" }; | ||
if (output_file_.find(listen, output_file_.length() - listen.length()) == std::string::npos) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just slightly wondering whether it's necessary to constrain the length of the search here, wouldn't that happen by itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, I can get rid of the length constraint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, this is the equivalent of ends_with(), so the constraint here is the start position of the search.
My minor whinging notwithstanding, I'm happy to merge...! |
If the libav output format is not set, try and better guess what is required by looking to see if the output is a tcp:// or udp:// url. In such cases, use a h264 elementray stream output format, assuming we are encoding with an appropriate H.264 encoder. Signed-off-by: Naushir Patuck <[email protected]>
If the --listen command line argument is set, ensure the url has the form "tcp://x.x.x.x:ABCD?listen=1" as expected by libav. Signed-off-by: Naushir Patuck <[email protected]>
No description provided.