| Message ID | 20210825161303.48955-2-jacopo@jmondi.org |
|---|---|
| State | RFC |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Wed, Aug 25, 2021 at 06:13:02PM +0200, Jacopo Mondi wrote: > The documentation suggests to use CameraConfiguration::operator[] to > access the StreamConfiguration it contains, but as CameraConfiguration > instances are generated by the Camera class and are returned wrapped in > a unique_ptr<>. The usage of operator[] would require an awkward syntax such > as (*config)[i]. > > Better to suggest the usage of the CameraConfiguration::at() function > instead to access the StreamConfigurations. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/camera.cpp | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index c20e05014fb9..c4d576b3985b 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -114,9 +114,9 @@ LOG_DECLARE_CATEGORY(Camera) > * The CameraConfiguration holds an ordered list of stream configurations. It > * supports iterators and operates as a vector of StreamConfiguration instances. > * The stream configurations are inserted by addConfiguration(), and the > - * operator[](int) returns a reference to the StreamConfiguration based on its > - * insertion index. Accessing a stream configuration with an invalid index > - * results in undefined behaviour. > + * at(unsigned int) function returns a reference to the StreamConfiguration We could say "at(unsigned int) function or operator[](int)" too. I'm fine with either. Another small comment, I'd drop the type and just write at() (and possibly operator[]()) as that's unambiguous. With or without this, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + * based on its insertion index. Accessing a stream configuration with an > + * invalid index results in undefined behaviour. > * > * CameraConfiguration instances are retrieved from the camera with > * Camera::generateConfiguration(). Applications may then inspect the
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index c20e05014fb9..c4d576b3985b 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -114,9 +114,9 @@ LOG_DECLARE_CATEGORY(Camera) * The CameraConfiguration holds an ordered list of stream configurations. It * supports iterators and operates as a vector of StreamConfiguration instances. * The stream configurations are inserted by addConfiguration(), and the - * operator[](int) returns a reference to the StreamConfiguration based on its - * insertion index. Accessing a stream configuration with an invalid index - * results in undefined behaviour. + * at(unsigned int) function returns a reference to the StreamConfiguration + * based on its insertion index. Accessing a stream configuration with an + * invalid index results in undefined behaviour. * * CameraConfiguration instances are retrieved from the camera with * Camera::generateConfiguration(). Applications may then inspect the
The documentation suggests to use CameraConfiguration::operator[] to access the StreamConfiguration it contains, but as CameraConfiguration instances are generated by the Camera class and are returned wrapped in a unique_ptr<>. The usage of operator[] would require an awkward syntax such as (*config)[i]. Better to suggest the usage of the CameraConfiguration::at() function instead to access the StreamConfigurations. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/camera.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)