| Message ID | 20241125153003.3309066-5-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Tue, Nov 26, 2024 at 12:30:03AM +0900, Paul Elder wrote: > Now that there is support for retrieving the allowed directions of a > control, print this information when listing controls. > > Sample output: > $ cam --list-controls -c 2 > Using camera Virtual0 as cam0 > Control: [io] draft::FaceDetectMode: > - FaceDetectModeOff (0) > Control: [i ] libcamera::FrameDurationLimits: [16666..33333] I was expecting [in], [out] or [input], but i/o has the advantage of aligning the controls names. We could still achieve the same with [in ] [ out] [inout] at the cost of longer lines, or '[in] ' '[out] ' '[inout]' any opinion from anyone ? > Size: 2 > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/apps/cam/camera_session.cpp | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp > index 6e9890ccfda1..1baf36fcfd3c 100644 > --- a/src/apps/cam/camera_session.cpp > +++ b/src/apps/cam/camera_session.cpp > @@ -159,12 +159,18 @@ CameraSession::~CameraSession() > void CameraSession::listControls() const > { > for (const auto &[id, info] : camera_->controls()) { > + std::stringstream io; > + io << "[" > + << (id->isInput() ? "i" : " ") > + << (id->isOutput() ? "o" : " ") > + << "] "; I wonder if a stringstream is overkill here, maybe you could fold the "[" and "]" in the statements below, and just construct a string here. It also depends on the outcome of the above discussion. Up to you. > + > if (info.values().empty()) { > - std::cout << "Control: " > + std::cout << "Control: " << io.str() > << id->vendor() << "::" << id->name() << ": " > << info.toString() << std::endl; > } else { > - std::cout << "Control: " > + std::cout << "Control: " << io.str() > << id->vendor() << "::" << id->name() << ":" > << std::endl; > for (const auto &value : info.values()) {
On Tue, Nov 26, 2024 at 01:22:47AM +0200, Laurent Pinchart wrote: > Hi Paul, > > Thank you for the patch. > > On Tue, Nov 26, 2024 at 12:30:03AM +0900, Paul Elder wrote: > > Now that there is support for retrieving the allowed directions of a > > control, print this information when listing controls. > > > > Sample output: > > $ cam --list-controls -c 2 > > Using camera Virtual0 as cam0 > > Control: [io] draft::FaceDetectMode: > > - FaceDetectModeOff (0) > > Control: [i ] libcamera::FrameDurationLimits: [16666..33333] > > I was expecting [in], [out] or [input], but i/o has the advantage of > aligning the controls names. We could still achieve the same with > > [in ] > [ out] > [inout] Yeah I'll expand it to this at least. Single-characters are indeed cryptic. > > at the cost of longer lines, or > > '[in] ' > '[out] ' > '[inout]' > > any opinion from anyone ? > > > Size: 2 > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > src/apps/cam/camera_session.cpp | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp > > index 6e9890ccfda1..1baf36fcfd3c 100644 > > --- a/src/apps/cam/camera_session.cpp > > +++ b/src/apps/cam/camera_session.cpp > > @@ -159,12 +159,18 @@ CameraSession::~CameraSession() > > void CameraSession::listControls() const > > { > > for (const auto &[id, info] : camera_->controls()) { > > + std::stringstream io; > > + io << "[" > > + << (id->isInput() ? "i" : " ") > > + << (id->isOutput() ? "o" : " ") > > + << "] "; > > I wonder if a stringstream is overkill here, maybe you could fold the > "[" and "]" in the statements below, and just construct a string here. > It also depends on the outcome of the above discussion. Up to you. I'd rather not duplicate this twice below... Paul > > > + > > if (info.values().empty()) { > > - std::cout << "Control: " > > + std::cout << "Control: " << io.str() > > << id->vendor() << "::" << id->name() << ": " > > << info.toString() << std::endl; > > } else { > > - std::cout << "Control: " > > + std::cout << "Control: " << io.str() > > << id->vendor() << "::" << id->name() << ":" > > << std::endl; > > for (const auto &value : info.values()) {
diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp index 6e9890ccfda1..1baf36fcfd3c 100644 --- a/src/apps/cam/camera_session.cpp +++ b/src/apps/cam/camera_session.cpp @@ -159,12 +159,18 @@ CameraSession::~CameraSession() void CameraSession::listControls() const { for (const auto &[id, info] : camera_->controls()) { + std::stringstream io; + io << "[" + << (id->isInput() ? "i" : " ") + << (id->isOutput() ? "o" : " ") + << "] "; + if (info.values().empty()) { - std::cout << "Control: " + std::cout << "Control: " << io.str() << id->vendor() << "::" << id->name() << ": " << info.toString() << std::endl; } else { - std::cout << "Control: " + std::cout << "Control: " << io.str() << id->vendor() << "::" << id->name() << ":" << std::endl; for (const auto &value : info.values()) {
Now that there is support for retrieving the allowed directions of a control, print this information when listing controls. Sample output: $ cam --list-controls -c 2 Using camera Virtual0 as cam0 Control: [io] draft::FaceDetectMode: - FaceDetectModeOff (0) Control: [i ] libcamera::FrameDurationLimits: [16666..33333] Size: 2 Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- src/apps/cam/camera_session.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)