| Message ID | 20210313102741.2890809-3-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, Thank you for the patch. On Sat, Mar 13, 2021 at 10:27:41AM +0000, Naushir Patuck wrote: > If the ISP::Output1 stream has not been configured, we must enable it Did you mean Output0 ? > with a default format and resolution for internal use. This is to allow > the pipeline handler data flow to be consistent, and allow the IPA to > receive statistics for the frame. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > .../pipeline/raspberrypi/raspberrypi.cpp | 30 ++++++++++++++++++- > 1 file changed, 29 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 9847e926b048..5cc04ce4eb92 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -625,7 +625,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > * StreamConfiguration appropriately. > */ > V4L2DeviceFormat format; > - bool output1Set = false; > + bool output0Set = false, output1Set = false; > for (unsigned i = 0; i < config->size(); i++) { > StreamConfiguration &cfg = config->at(i); > > @@ -662,6 +662,34 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > > if (i != maxIndex) > output1Set = true; > + else > + output0Set = true; > + } > + > + /* > + * If ISP::Output0 stream has not been configured by the application, > + * we must allow the hardware to generate an output so that the data > + * flow in the pipeline handler remains consistent, and we still generate > + * statistics for the IPA to use. So enable the output at a very low > + * resolution for internal use. > + * > + * \todo Allow the pipeline to work correctly without Output0 and only > + * statistics coming from the hardware. > + */ > + if (!output0Set) { > + maxSize = Size(320, 240); > + format = {}; > + format.size = maxSize; > + format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420, false); > + ret = data->isp_[Isp::Output0].dev()->setFormat(&format); > + if (ret) { > + LOG(RPI, Error) << "Failed to set default format on ISP Output0: " > + << ret; Minor comment, to shorten the lines, LOG(RPI, Error) << "Failed to set default format on ISP Output0: " << ret; Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I'll apply this after a confirmation that the commit message should mention Ouput0. > + return -EINVAL; > + } > + > + LOG(RPI, Debug) << "Defaulting ISP Output0 format to " > + << format.toString(); > } > > /*
Hi Laurent, On Sat, 13 Mar 2021 at 20:16, Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hi Naush, > > Thank you for the patch. > > On Sat, Mar 13, 2021 at 10:27:41AM +0000, Naushir Patuck wrote: > > If the ISP::Output1 stream has not been configured, we must enable it > > Did you mean Output0 ? > > > with a default format and resolution for internal use. This is to allow > > the pipeline handler data flow to be consistent, and allow the IPA to > > receive statistics for the frame. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > .../pipeline/raspberrypi/raspberrypi.cpp | 30 ++++++++++++++++++- > > 1 file changed, 29 insertions(+), 1 deletion(-) > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > index 9847e926b048..5cc04ce4eb92 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -625,7 +625,7 @@ int PipelineHandlerRPi::configure(Camera *camera, > CameraConfiguration *config) > > * StreamConfiguration appropriately. > > */ > > V4L2DeviceFormat format; > > - bool output1Set = false; > > + bool output0Set = false, output1Set = false; > > for (unsigned i = 0; i < config->size(); i++) { > > StreamConfiguration &cfg = config->at(i); > > > > @@ -662,6 +662,34 @@ int PipelineHandlerRPi::configure(Camera *camera, > CameraConfiguration *config) > > > > if (i != maxIndex) > > output1Set = true; > > + else > > + output0Set = true; > > + } > > + > > + /* > > + * If ISP::Output0 stream has not been configured by the > application, > > + * we must allow the hardware to generate an output so that the > data > > + * flow in the pipeline handler remains consistent, and we still > generate > > + * statistics for the IPA to use. So enable the output at a very > low > > + * resolution for internal use. > > + * > > + * \todo Allow the pipeline to work correctly without Output0 and > only > > + * statistics coming from the hardware. > > + */ > > + if (!output0Set) { > > + maxSize = Size(320, 240); > > + format = {}; > > + format.size = maxSize; > > + format.fourcc = > V4L2PixelFormat::fromPixelFormat(formats::YUV420, false); > > + ret = data->isp_[Isp::Output0].dev()->setFormat(&format); > > + if (ret) { > > + LOG(RPI, Error) << "Failed to set default format > on ISP Output0: " > > + << ret; > > Minor comment, to shorten the lines, > > LOG(RPI, Error) > << "Failed to set default format on ISP > Output0: " > << ret; > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I'll apply this after a confirmation that the commit message should > mention Ouput0. > Quite right, that should be Output0. Thank you for correcting that! Regards, Naush > > > + return -EINVAL; > > + } > > + > > + LOG(RPI, Debug) << "Defaulting ISP Output0 format to " > > + << format.toString(); > > } > > > > /* > > -- > Regards, > > Laurent Pinchart >
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 9847e926b048..5cc04ce4eb92 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -625,7 +625,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) * StreamConfiguration appropriately. */ V4L2DeviceFormat format; - bool output1Set = false; + bool output0Set = false, output1Set = false; for (unsigned i = 0; i < config->size(); i++) { StreamConfiguration &cfg = config->at(i); @@ -662,6 +662,34 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) if (i != maxIndex) output1Set = true; + else + output0Set = true; + } + + /* + * If ISP::Output0 stream has not been configured by the application, + * we must allow the hardware to generate an output so that the data + * flow in the pipeline handler remains consistent, and we still generate + * statistics for the IPA to use. So enable the output at a very low + * resolution for internal use. + * + * \todo Allow the pipeline to work correctly without Output0 and only + * statistics coming from the hardware. + */ + if (!output0Set) { + maxSize = Size(320, 240); + format = {}; + format.size = maxSize; + format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420, false); + ret = data->isp_[Isp::Output0].dev()->setFormat(&format); + if (ret) { + LOG(RPI, Error) << "Failed to set default format on ISP Output0: " + << ret; + return -EINVAL; + } + + LOG(RPI, Debug) << "Defaulting ISP Output0 format to " + << format.toString(); } /*
If the ISP::Output1 stream has not been configured, we must enable it with a default format and resolution for internal use. This is to allow the pipeline handler data flow to be consistent, and allow the IPA to receive statistics for the frame. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- .../pipeline/raspberrypi/raspberrypi.cpp | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)