core: adding method to access CameraManager #528
Conversation
There was a problem hiding this comment.
This changes seems reasonable, so I am happy to merge something along these lines. However, from our discussion on #527, you will want to pass this pointer into LibcameraApp::GetCameras() which takes in a const std::unique_ptr<CameraManager> & as a parameter. Perhaps this function also needs to change to take in a const libcamera::CameraManager * instead?
core/libcamera_app.cpp
Outdated
| @@ -773,6 +773,11 @@ libcamera::Stream *LibcameraApp::GetMainStream() const | |||
| return nullptr; | |||
| } | |||
|
|
|||
| libcamera::CameraManager *LibcameraApp::GetCameraManager() const | |||
There was a problem hiding this comment.
Perhaps this function should return a const libcamera::CameraManager * value?
There was a problem hiding this comment.
sure. as I said, my c++ is stuck in the late 90s...
There was a problem hiding this comment.
This ought to change to return const libcamera::CameraManager *
|
One more thing, could you add a small description to the commit message, and end it with a |
anjok
force-pushed
the
ak/camera_manager
branch
2 times, most recently
from
3aaaec2 to
ac5a0f9
Compare
There was a problem hiding this comment.
Additionally, in the commit message, could you please use your name and email on the Signed-off-by line?
Signed-off-by: Your Name <[email protected]>
core/libcamera_app.cpp
Outdated
| @@ -773,6 +773,11 @@ libcamera::Stream *LibcameraApp::GetMainStream() const | |||
| return nullptr; | |||
| } | |||
|
|
|||
| libcamera::CameraManager *LibcameraApp::GetCameraManager() const | |||
There was a problem hiding this comment.
This ought to change to return const libcamera::CameraManager *
| { | ||
| return GetCameras(camera_manager_); | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't think the GetCameras() override is needed right? Instead you need to change (below):
static std::vector<std::shared_ptr<libcamera::Camera>> GetCameras(const std::unique_ptr<CameraManager> &cm)
to
static std::vector<std::shared_ptr<libcamera::Camera>> GetCameras(const CameraManager> *cm)
There was a problem hiding this comment.
right.
But I already have a LibCameraApp, so I might as well just use a new app->GetCameras() off of it and not deal with the camera manager at all? After all, I was only doing that because that's what the multiple other code pieces in LibCameraApp did and you didn't have a singular means to get at the list.
Also, you'd need to change "public", if interim, API.. I don't care one way or another, but given there's no way to get at the manager without an app in the first place, and there's not way to get a unique_ptr without having one first, I would recommend (again without knowing a lot about what all of this does or should do) to remove the static method and just have an instance method....
There was a problem hiding this comment.
OK, so the options use that method... why? I guess this is where our problems came from, as we're using the options when the app has already been created. So now you have two cam managers... at least I think so.
There was a problem hiding this comment.
There is another place that uses GetCameras() with the camera manager already opened. This is why passing in an external camera manager must still be supported.
Added the ability to get at the CameraManager to allow for access to the Camera list. Also addded instance method to get the Camera list. Signed-off-by: Anjo Krank <[email protected]>
Fixed... but I was under the impression that this would be the committer who said "this is ok to merge"? Why can I sign off my own stuff as an external? |
The Signed-off-by tag is signed by the author/submitter of the commit for traceability reasons. |
one way to fix issue #527