| Message ID | 20230614121332.15484-1-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Commit | 0d1e402e1b6d0fce1e0003bf3deebb21594d5906 |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush On Wed, Jun 14, 2023 at 01:13:32PM +0100, Naushir Patuck via libcamera-devel wrote: > If a user provided pipeline config file is not present, or if the > version reported in the file is invalid, do not return with an error > when creating the pipeline handler. Instead, log a warning message and > return success with default pipeline config values used. > > This now matches the behaviour when the pipeline config file could not > be parsed correctly, and we revert to default values. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> I would have gone for the other way around, and make the Warning an Error, as if someone specifies a config file, it should be a correct one. Anyway, matter of tastes I guess, consistency is what matters here Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > --- > src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > index 1e20fc2d8cb8..df7482920e75 100644 > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > @@ -1093,8 +1093,9 @@ int CameraData::loadPipelineConfiguration() > File file(filename); > > if (!file.open(File::OpenModeFlag::ReadOnly)) { > - LOG(RPI, Error) << "Failed to open configuration file '" << filename << "'"; > - return -EIO; > + LOG(RPI, Warning) << "Failed to open configuration file '" << filename << "'" > + << ", using defaults"; > + return 0; > } > > LOG(RPI, Info) << "Using configuration file '" << filename << "'"; > @@ -1107,8 +1108,9 @@ int CameraData::loadPipelineConfiguration() > > std::optional<double> ver = (*root)["version"].get<double>(); > if (!ver || *ver != 1.0) { > - LOG(RPI, Error) << "Unexpected configuration file version reported"; > - return -EINVAL; > + LOG(RPI, Warning) << "Unexpected configuration file version reported: " > + << *ver; > + return 0; > } > > const YamlObject &phConfig = (*root)["pipeline_handler"]; > -- > 2.34.1 >
Hi Naush Thanks for the patch. On Wed, 14 Jun 2023 at 13:13, Naushir Patuck <naush@raspberrypi.com> wrote: > > If a user provided pipeline config file is not present, or if the > version reported in the file is invalid, do not return with an error > when creating the pipeline handler. Instead, log a warning message and > return success with default pipeline config values used. > > This now matches the behaviour when the pipeline config file could not > be parsed correctly, and we revert to default values. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> Yes, I think I prefer this too for our users. Reviewed-by: David Plowman <david.plowman@raspberrypi.com> Thanks! David > --- > src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > index 1e20fc2d8cb8..df7482920e75 100644 > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > @@ -1093,8 +1093,9 @@ int CameraData::loadPipelineConfiguration() > File file(filename); > > if (!file.open(File::OpenModeFlag::ReadOnly)) { > - LOG(RPI, Error) << "Failed to open configuration file '" << filename << "'"; > - return -EIO; > + LOG(RPI, Warning) << "Failed to open configuration file '" << filename << "'" > + << ", using defaults"; > + return 0; > } > > LOG(RPI, Info) << "Using configuration file '" << filename << "'"; > @@ -1107,8 +1108,9 @@ int CameraData::loadPipelineConfiguration() > > std::optional<double> ver = (*root)["version"].get<double>(); > if (!ver || *ver != 1.0) { > - LOG(RPI, Error) << "Unexpected configuration file version reported"; > - return -EINVAL; > + LOG(RPI, Warning) << "Unexpected configuration file version reported: " > + << *ver; > + return 0; > } > > const YamlObject &phConfig = (*root)["pipeline_handler"]; > -- > 2.34.1 >
diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index 1e20fc2d8cb8..df7482920e75 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -1093,8 +1093,9 @@ int CameraData::loadPipelineConfiguration() File file(filename); if (!file.open(File::OpenModeFlag::ReadOnly)) { - LOG(RPI, Error) << "Failed to open configuration file '" << filename << "'"; - return -EIO; + LOG(RPI, Warning) << "Failed to open configuration file '" << filename << "'" + << ", using defaults"; + return 0; } LOG(RPI, Info) << "Using configuration file '" << filename << "'"; @@ -1107,8 +1108,9 @@ int CameraData::loadPipelineConfiguration() std::optional<double> ver = (*root)["version"].get<double>(); if (!ver || *ver != 1.0) { - LOG(RPI, Error) << "Unexpected configuration file version reported"; - return -EINVAL; + LOG(RPI, Warning) << "Unexpected configuration file version reported: " + << *ver; + return 0; } const YamlObject &phConfig = (*root)["pipeline_handler"];
If a user provided pipeline config file is not present, or if the version reported in the file is invalid, do not return with an error when creating the pipeline handler. Instead, log a warning message and return success with default pipeline config values used. This now matches the behaviour when the pipeline config file could not be parsed correctly, and we revert to default values. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)