| Message ID | 20210318125521.43278-2-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Thu, Mar 18, 2021 at 01:55:20PM +0100, Jacopo Mondi wrote: > Do not register the Location property if not available from the firmware > interface instead of defaulting it to External. This looks good, but I think we need a patch before this one to make the location property optional in the cam application. > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/camera_sensor.cpp | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 27f82071151e..f7ed91d990f7 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -468,12 +468,10 @@ int CameraSensor::initProperties() > propertyValue = properties::CameraLocationBack; > break; > } > + properties_.set(properties::Location, propertyValue); > } else { > - LOG(CameraSensor, Warning) > - << "Failed to retrieve the camera location, setting to External"; > - propertyValue = properties::CameraLocationExternal; > + LOG(CameraSensor, Warning) << "Failed to retrieve the camera location"; > } > - properties_.set(properties::Location, propertyValue); > > const auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION); > if (rotationControl != controls.end()) {
Hi Laurent, On Thu, Mar 18, 2021 at 03:01:07PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Thu, Mar 18, 2021 at 01:55:20PM +0100, Jacopo Mondi wrote: > > Do not register the Location property if not available from the firmware > > interface instead of defaulting it to External. > > This looks good, but I think we need a patch before this one to make the > location property optional in the cam application. I'm so concerned about CTS I've ignored our own test app :/ How should it look like, as now Location is used to build the camera name. Simply remove any reference to the camera position ? Thanks j > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > src/libcamera/camera_sensor.cpp | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > > index 27f82071151e..f7ed91d990f7 100644 > > --- a/src/libcamera/camera_sensor.cpp > > +++ b/src/libcamera/camera_sensor.cpp > > @@ -468,12 +468,10 @@ int CameraSensor::initProperties() > > propertyValue = properties::CameraLocationBack; > > break; > > } > > + properties_.set(properties::Location, propertyValue); > > } else { > > - LOG(CameraSensor, Warning) > > - << "Failed to retrieve the camera location, setting to External"; > > - propertyValue = properties::CameraLocationExternal; > > + LOG(CameraSensor, Warning) << "Failed to retrieve the camera location"; > > } > > - properties_.set(properties::Location, propertyValue); > > > > const auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION); > > if (rotationControl != controls.end()) { > > -- > Regards, > > Laurent Pinchart
Hi Jacopo, On Thu, Mar 18, 2021 at 02:41:09PM +0100, Jacopo Mondi wrote: > On Thu, Mar 18, 2021 at 03:01:07PM +0200, Laurent Pinchart wrote: > > On Thu, Mar 18, 2021 at 01:55:20PM +0100, Jacopo Mondi wrote: > > > Do not register the Location property if not available from the firmware > > > interface instead of defaulting it to External. > > > > This looks good, but I think we need a patch before this one to make the > > location property optional in the cam application. > > I'm so concerned about CTS I've ignored our own test app :/ > How should it look like, as now Location is used to build the camera > name. Simply remove any reference to the camera position ? I'd make it optional, keeping it in the name if it's present, removing it otherwise. > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > --- > > > src/libcamera/camera_sensor.cpp | 6 ++---- > > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > > > index 27f82071151e..f7ed91d990f7 100644 > > > --- a/src/libcamera/camera_sensor.cpp > > > +++ b/src/libcamera/camera_sensor.cpp > > > @@ -468,12 +468,10 @@ int CameraSensor::initProperties() > > > propertyValue = properties::CameraLocationBack; > > > break; > > > } > > > + properties_.set(properties::Location, propertyValue); > > > } else { > > > - LOG(CameraSensor, Warning) > > > - << "Failed to retrieve the camera location, setting to External"; > > > - propertyValue = properties::CameraLocationExternal; > > > + LOG(CameraSensor, Warning) << "Failed to retrieve the camera location"; > > > } > > > - properties_.set(properties::Location, propertyValue); > > > > > > const auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION); > > > if (rotationControl != controls.end()) {
diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 27f82071151e..f7ed91d990f7 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -468,12 +468,10 @@ int CameraSensor::initProperties() propertyValue = properties::CameraLocationBack; break; } + properties_.set(properties::Location, propertyValue); } else { - LOG(CameraSensor, Warning) - << "Failed to retrieve the camera location, setting to External"; - propertyValue = properties::CameraLocationExternal; + LOG(CameraSensor, Warning) << "Failed to retrieve the camera location"; } - properties_.set(properties::Location, propertyValue); const auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION); if (rotationControl != controls.end()) {
Do not register the Location property if not available from the firmware interface instead of defaulting it to External. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/camera_sensor.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)