Message ID | 20200720091311.805092-8-naush@raspberrypi.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Naush, On 20/07/2020 10:13, Naushir Patuck wrote: > The counter was not incremented, so multiple streams would only pass the > last stream config to the IPA. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index dbc22521..c28fe997 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -976,14 +976,14 @@ int RPiCameraData::configureIPA() > unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); > /* Inform IPA of stream configuration and sensor controls. */ > unsigned int i = 0; > - for (auto const &stream : isp_) { > + for (auto const &stream : isp_) > if (stream.isExternal()) { > - streamConfig[i] = { > + streamConfig[i++] = { > .pixelFormat = stream.configuration().pixelFormat, > .size = stream.configuration().size > }; > } > - } > + Hrm, I would have probably kept the outer scope braces on the for loop, it only has a single scope statement, but it's more complex than a single line. Not sure if we have a defined rule on this for style, but I don't object anyway... > entityControls.emplace(0, unicam_[Unicam::Image].dev()->controls()); > entityControls.emplace(1, isp_[Isp::Input].dev()->controls()); > >
Hi Kieran, On Tue, 21 Jul 2020 at 12:14, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Hi Naush, > > On 20/07/2020 10:13, Naushir Patuck wrote: > > The counter was not incremented, so multiple streams would only pass the > > last stream config to the IPA. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > --- > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > index dbc22521..c28fe997 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -976,14 +976,14 @@ int RPiCameraData::configureIPA() > > unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); > > /* Inform IPA of stream configuration and sensor controls. */ > > unsigned int i = 0; > > - for (auto const &stream : isp_) { > > + for (auto const &stream : isp_) > > if (stream.isExternal()) { > > - streamConfig[i] = { > > + streamConfig[i++] = { > > .pixelFormat = stream.configuration().pixelFormat, > > .size = stream.configuration().size > > }; > > } > > - } > > + > > Hrm, I would have probably kept the outer scope braces on the for loop, > it only has a single scope statement, but it's more complex than a > single line. > > Not sure if we have a defined rule on this for style, but I don't object > anyway... Was not sure what the accepted format was. I will likely submit a new version of the patchset, so will add back the outer scope brackets. Regards, Naush > > > > entityControls.emplace(0, unicam_[Unicam::Image].dev()->controls()); > > entityControls.emplace(1, isp_[Isp::Input].dev()->controls()); > > > > > > -- > Regards > -- > Kieran
Hi Naush, Thank you for the patch. On Wed, Jul 22, 2020 at 08:54:41AM +0100, Naushir Patuck wrote: > On Tue, 21 Jul 2020 at 12:14, Kieran Bingham wrote: > > On 20/07/2020 10:13, Naushir Patuck wrote: > > > The counter was not incremented, so multiple streams would only pass the > > > last stream config to the IPA. > > > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > --- > > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > index dbc22521..c28fe997 100644 > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > @@ -976,14 +976,14 @@ int RPiCameraData::configureIPA() > > > unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); > > > /* Inform IPA of stream configuration and sensor controls. */ > > > unsigned int i = 0; > > > - for (auto const &stream : isp_) { > > > + for (auto const &stream : isp_) > > > if (stream.isExternal()) { > > > - streamConfig[i] = { > > > + streamConfig[i++] = { > > > .pixelFormat = stream.configuration().pixelFormat, > > > .size = stream.configuration().size > > > }; > > > } > > > - } > > > + > > > > Hrm, I would have probably kept the outer scope braces on the for loop, > > it only has a single scope statement, but it's more complex than a > > single line. > > > > Not sure if we have a defined rule on this for style, but I don't object > > anyway... > > Was not sure what the accepted format was. I will likely submit a new > version of the patchset, so will add back the outer scope brackets. Yes, that's the preferred style. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Even though it likely won't make much of a difference, as the IPA doesn't seem to use streamConfig :-) > > > entityControls.emplace(0, unicam_[Unicam::Image].dev()->controls()); > > > entityControls.emplace(1, isp_[Isp::Input].dev()->controls()); > > >
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index dbc22521..c28fe997 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -976,14 +976,14 @@ int RPiCameraData::configureIPA() unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); /* Inform IPA of stream configuration and sensor controls. */ unsigned int i = 0; - for (auto const &stream : isp_) { + for (auto const &stream : isp_) if (stream.isExternal()) { - streamConfig[i] = { + streamConfig[i++] = { .pixelFormat = stream.configuration().pixelFormat, .size = stream.configuration().size }; } - } + entityControls.emplace(0, unicam_[Unicam::Image].dev()->controls()); entityControls.emplace(1, isp_[Isp::Input].dev()->controls());