Message ID | 20250130074712.959046-1-pobrn@protonmail.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Quoting Barnabás Pőcze (2025-01-30 07:47:15) > The mentioned commit mistakenly introduced a new variable for storing > the camera instead of just assigining to the global variable defined > earlier in the tutorial. Fix that by making it an assignment. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=252 > Fixes: e77a2751100e38 ("treewide: Query list of cameras just once") > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > --- > Documentation/guides/application-developer.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst > index 67f5bd7e7..297586a1c 100644 > --- a/Documentation/guides/application-developer.rst > +++ b/Documentation/guides/application-developer.rst > @@ -128,7 +128,7 @@ available. > > std::string cameraId = cameras[0]->id(); > > - auto camera = cm->get(cameraId); > + camera = cm->get(cameraId); I always thought we should be providing a request->camera() helper to return the associated camera with a request to make this easier for applciations but it didn't seem to get any traction when I posted this: https://patchwork.libcamera.org/patch/9328/ So for this patch indeed this stops the aliased variable. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > /* > * Note that `camera` may not compare equal to `cameras[0]`. > * In fact, it might simply be a `nullptr`, as the particular > -- > 2.48.1 > >
2025. január 30., csütörtök 10:04 keltezéssel, Kieran Bingham <kieran.bingham@ideasonboard.com> írta: > Quoting Barnabás Pőcze (2025-01-30 07:47:15) > > The mentioned commit mistakenly introduced a new variable for storing > > the camera instead of just assigining to the global variable defined > > earlier in the tutorial. Fix that by making it an assignment. > > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=252 > > Fixes: e77a2751100e38 ("treewide: Query list of cameras just once") > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > --- > > Documentation/guides/application-developer.rst | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst > > index 67f5bd7e7..297586a1c 100644 > > --- a/Documentation/guides/application-developer.rst > > +++ b/Documentation/guides/application-developer.rst > > @@ -128,7 +128,7 @@ available. > > > > std::string cameraId = cameras[0]->id(); > > > > - auto camera = cm->get(cameraId); > > + camera = cm->get(cameraId); > > I always thought we should be providing a request->camera() helper to > return the associated camera with a request to make this easier for > applciations but it didn't seem to get any traction when I posted this: > > https://patchwork.libcamera.org/patch/9328/ I see the camera pointer has been moved to `Request::Private`, but I think this still seems like a good idea. Although I have no idea what the mentioned plans with `Request` were. Regards, Barnabás Pőcze > > So for this patch indeed this stops the aliased variable. > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > /* > > * Note that `camera` may not compare equal to `cameras[0]`. > > * In fact, it might simply be a `nullptr`, as the particular > > -- > > 2.48.1 > > > > >
Quoting Barnabás Pőcze (2025-01-30 10:37:22) > 2025. január 30., csütörtök 10:04 keltezéssel, Kieran Bingham <kieran.bingham@ideasonboard.com> írta: > > > Quoting Barnabás Pőcze (2025-01-30 07:47:15) > > > The mentioned commit mistakenly introduced a new variable for storing > > > the camera instead of just assigining to the global variable defined > > > earlier in the tutorial. Fix that by making it an assignment. > > > > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=252 > > > Fixes: e77a2751100e38 ("treewide: Query list of cameras just once") > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > > --- > > > Documentation/guides/application-developer.rst | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst > > > index 67f5bd7e7..297586a1c 100644 > > > --- a/Documentation/guides/application-developer.rst > > > +++ b/Documentation/guides/application-developer.rst > > > @@ -128,7 +128,7 @@ available. > > > > > > std::string cameraId = cameras[0]->id(); > > > > > > - auto camera = cm->get(cameraId); > > > + camera = cm->get(cameraId); > > > > I always thought we should be providing a request->camera() helper to > > return the associated camera with a request to make this easier for > > applciations but it didn't seem to get any traction when I posted this: > > > > https://patchwork.libcamera.org/patch/9328/ > > I see the camera pointer has been moved to `Request::Private`, but I think > this still seems like a good idea. Although I have no idea what the mentioned > plans with `Request` were. I don't know what the resistance context was either, but I still think it seems like a useful way to access the camera in the async delivery especially as the camera is already stored /in/ the request and is guaranteed to be correctly associated. Maybe it's worth refreshing the patch to see. -- Kieran > > Regards, > Barnabás Pőcze > > > > > So for this patch indeed this stops the aliased variable. > > > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > > > /* > > > * Note that `camera` may not compare equal to `cameras[0]`. > > > * In fact, it might simply be a `nullptr`, as the particular > > > -- > > > 2.48.1 > > > > > > > >
Hi Barnabás, Thank you for the patch. On Thu, Jan 30, 2025 at 07:47:15AM +0000, Barnabás Pőcze wrote: > The mentioned commit mistakenly introduced a new variable for storing > the camera instead of just assigining to the global variable defined > earlier in the tutorial. Fix that by making it an assignment. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=252 > Fixes: e77a2751100e38 ("treewide: Query list of cameras just once") > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > Documentation/guides/application-developer.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst > index 67f5bd7e7..297586a1c 100644 > --- a/Documentation/guides/application-developer.rst > +++ b/Documentation/guides/application-developer.rst > @@ -128,7 +128,7 @@ available. > > std::string cameraId = cameras[0]->id(); > > - auto camera = cm->get(cameraId); > + camera = cm->get(cameraId); > /* > * Note that `camera` may not compare equal to `cameras[0]`. > * In fact, it might simply be a `nullptr`, as the particular
diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst index 67f5bd7e7..297586a1c 100644 --- a/Documentation/guides/application-developer.rst +++ b/Documentation/guides/application-developer.rst @@ -128,7 +128,7 @@ available. std::string cameraId = cameras[0]->id(); - auto camera = cm->get(cameraId); + camera = cm->get(cameraId); /* * Note that `camera` may not compare equal to `cameras[0]`. * In fact, it might simply be a `nullptr`, as the particular
The mentioned commit mistakenly introduced a new variable for storing the camera instead of just assigining to the global variable defined earlier in the tutorial. Fix that by making it an assignment. Bug: https://bugs.libcamera.org/show_bug.cgi?id=252 Fixes: e77a2751100e38 ("treewide: Query list of cameras just once") Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> --- Documentation/guides/application-developer.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)