Message ID | 20210810075854.86191-2-umang.jain@ideasonboard.com |
---|---|
State | Superseded |
Delegated to: | Umang Jain |
Headers | show |
Series |
|
Related | show |
Hi Umang, On Tue, Aug 10, 2021 at 01:28:51PM +0530, Umang Jain wrote: > In CameraSensor, the mbusCodes() and sizes() accessor functions > retrieves all the supported media bus codes and the supported sizes > respectively. However, this is quite limiting since the caller > probably isn't in a position to match which range of sizes are > supported for a particular mbusCode. > > Hence, the caller is most likely interested to know about the sizes > supported for a particular media bus code. This patch transforms the > existing CameraSensor::sizes() to CameraSensor::sizes(mbuscode) to > achieve that goal. > > The patch also transforms existing CIO2Device::sizes() in IPU3 pipeline > handler to CIO2Device::sizes(PixelFormat) on a similar principle. The > function is then plumbed to CameraSensor::sizes(mbusCode) to enumerate > the per-format sizes as required in > PipelineHandlerIPU3::generateConfiguration(). > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > Tested-by: Umang Jain <umang.jain@ideasonboard.com> Well, I hope you test your patches before sending them. Do we need Tested-by tags from the author ? > Tested-by: Jacopo Mondi <jacopo@jmondi.org> I was sure I had reviewed the series in full I'm sorry, > --- > include/libcamera/internal/camera_sensor.h | 2 +- > src/libcamera/camera_sensor.cpp | 25 ++++++++++++++++------ > src/libcamera/pipeline/ipu3/cio2.cpp | 20 ++++++++++++----- > src/libcamera/pipeline/ipu3/cio2.h | 2 +- > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- > test/camera-sensor.cpp | 2 +- > 6 files changed, 38 insertions(+), 15 deletions(-) > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h > index db12b07e..d25a1165 100644 > --- a/include/libcamera/internal/camera_sensor.h > +++ b/include/libcamera/internal/camera_sensor.h > @@ -38,7 +38,7 @@ public: > const std::string &id() const { return id_; } > const MediaEntity *entity() const { return entity_; } > const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } > - const std::vector<Size> &sizes() const { return sizes_; } > + const std::vector<Size> sizes(unsigned int mbusCode) const; > Size resolution() const; > const std::vector<int32_t> &testPatternModes() const > { > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 7a386415..87668509 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -472,14 +472,27 @@ int CameraSensor::initProperties() > */ > > /** > - * \fn CameraSensor::sizes() > - * \brief Retrieve the frame sizes supported by the camera sensor > + * \brief Retrieve the supported frame sizes for a media bus code > + * \param[in] mbusCode The media bus code for which sizes are requested > * > - * The reported sizes span all media bus codes supported by the camera sensor. > - * Not all sizes may be supported by all media bus codes. > - * > - * \return The supported frame sizes sorted in increasing order > + * \return The supported frame sizes for \a mbusCode sorted in increasing order > */ > +const std::vector<Size> CameraSensor::sizes(unsigned int mbusCode) const > +{ > + std::vector<Size> sizes; > + > + const auto &format = formats_.find(mbusCode); > + if (format == formats_.end()) > + return sizes; > + > + const std::vector<SizeRange> &ranges = format->second; > + std::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes), > + [](const SizeRange &range) { return range.max; }); > + > + std::sort(sizes.begin(), sizes.end()); > + > + return sizes; > +} > > /** > * \brief Retrieve the camera sensor resolution > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp > index 1bcd580e..8a40e955 100644 > --- a/src/libcamera/pipeline/ipu3/cio2.cpp > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp > @@ -64,16 +64,26 @@ std::vector<PixelFormat> CIO2Device::formats() const > * \brief Retrieve the list of supported size ranges > * \return The list of supported SizeRange > */ > -std::vector<SizeRange> CIO2Device::sizes() const > +std::vector<SizeRange> CIO2Device::sizes(const PixelFormat &format) const I know we don't really doxygen, but as it's there, what about documenting it ? > { > + std::vector<SizeRange> szRange; Why have you moved it up ? I would also keep the same name > + int mbusCode = -1; > + > if (!sensor_) > return {}; > > - std::vector<SizeRange> sizes; > - for (const Size &size : sensor_->sizes()) > - sizes.emplace_back(size, size); > + for (const auto &iter : mbusCodesToPixelFormat) { > + if (iter.second == format) > + mbusCode = iter.first; There should be only one match, right ? Then if (iter.second != format) continue; mbusCode = iter.first; break; > + } > + > + if (!mbusCode) if (-1) evaluates to true With this fixed: Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > + return {}; > + > + for (const Size &sz: sensor_->sizes(mbusCode)) > + szRange.emplace_back(sz); > > - return sizes; > + return szRange; > } > > /** > diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h > index f28e9f1d..24272dc5 100644 > --- a/src/libcamera/pipeline/ipu3/cio2.h > +++ b/src/libcamera/pipeline/ipu3/cio2.h > @@ -35,7 +35,7 @@ public: > CIO2Device(); > > std::vector<PixelFormat> formats() const; > - std::vector<SizeRange> sizes() const; > + std::vector<SizeRange> sizes(const PixelFormat &format) const; > > int init(const MediaDevice *media, unsigned int index); > int configure(const Size &size, V4L2DeviceFormat *outputFormat); > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 19cb5c4e..c73540fb 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -455,7 +455,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, > bufferCount = cio2Config.bufferCount; > > for (const PixelFormat &format : data->cio2_.formats()) > - streamFormats[format] = data->cio2_.sizes(); > + streamFormats[format] = data->cio2_.sizes(format); > > break; > } > diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp > index a8dcad82..372ee4af 100644 > --- a/test/camera-sensor.cpp > +++ b/test/camera-sensor.cpp > @@ -76,7 +76,7 @@ protected: > return TestFail; > } > > - const std::vector<Size> &sizes = sensor_->sizes(); > + const std::vector<Size> &sizes = sensor_->sizes(*iter); > auto iter2 = std::find(sizes.begin(), sizes.end(), > Size(4096, 2160)); > if (iter2 == sizes.end()) { > -- > 2.31.1 >
Hi Umang, one additional comment On Wed, Aug 25, 2021 at 12:09:35PM +0200, Jacopo Mondi wrote: > Hi Umang, > > On Tue, Aug 10, 2021 at 01:28:51PM +0530, Umang Jain wrote: > > In CameraSensor, the mbusCodes() and sizes() accessor functions > > retrieves all the supported media bus codes and the supported sizes > > respectively. However, this is quite limiting since the caller > > probably isn't in a position to match which range of sizes are > > supported for a particular mbusCode. > > > > Hence, the caller is most likely interested to know about the sizes > > supported for a particular media bus code. This patch transforms the > > existing CameraSensor::sizes() to CameraSensor::sizes(mbuscode) to > > achieve that goal. > > > > The patch also transforms existing CIO2Device::sizes() in IPU3 pipeline > > handler to CIO2Device::sizes(PixelFormat) on a similar principle. The > > function is then plumbed to CameraSensor::sizes(mbusCode) to enumerate > > the per-format sizes as required in > > PipelineHandlerIPU3::generateConfiguration(). > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > Tested-by: Umang Jain <umang.jain@ideasonboard.com> > > Well, I hope you test your patches before sending them. Do we need > Tested-by tags from the author ? > > > Tested-by: Jacopo Mondi <jacopo@jmondi.org> > > I was sure I had reviewed the series in full I'm sorry, > > > > --- > > include/libcamera/internal/camera_sensor.h | 2 +- > > src/libcamera/camera_sensor.cpp | 25 ++++++++++++++++------ > > src/libcamera/pipeline/ipu3/cio2.cpp | 20 ++++++++++++----- > > src/libcamera/pipeline/ipu3/cio2.h | 2 +- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- > > test/camera-sensor.cpp | 2 +- > > 6 files changed, 38 insertions(+), 15 deletions(-) > > > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h > > index db12b07e..d25a1165 100644 > > --- a/include/libcamera/internal/camera_sensor.h > > +++ b/include/libcamera/internal/camera_sensor.h > > @@ -38,7 +38,7 @@ public: > > const std::string &id() const { return id_; } > > const MediaEntity *entity() const { return entity_; } > > const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } > > - const std::vector<Size> &sizes() const { return sizes_; } > > + const std::vector<Size> sizes(unsigned int mbusCode) const; > > Size resolution() const; > > const std::vector<int32_t> &testPatternModes() const > > { > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > > index 7a386415..87668509 100644 > > --- a/src/libcamera/camera_sensor.cpp > > +++ b/src/libcamera/camera_sensor.cpp > > @@ -472,14 +472,27 @@ int CameraSensor::initProperties() > > */ > > > > /** > > - * \fn CameraSensor::sizes() > > - * \brief Retrieve the frame sizes supported by the camera sensor > > + * \brief Retrieve the supported frame sizes for a media bus code > > + * \param[in] mbusCode The media bus code for which sizes are requested > > * > > - * The reported sizes span all media bus codes supported by the camera sensor. > > - * Not all sizes may be supported by all media bus codes. > > - * > > - * \return The supported frame sizes sorted in increasing order > > + * \return The supported frame sizes for \a mbusCode sorted in increasing order > > */ > > +const std::vector<Size> CameraSensor::sizes(unsigned int mbusCode) const > > +{ > > + std::vector<Size> sizes; > > + > > + const auto &format = formats_.find(mbusCode); > > + if (format == formats_.end()) > > + return sizes; > > + > > + const std::vector<SizeRange> &ranges = format->second; > > + std::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes), > > + [](const SizeRange &range) { return range.max; }); > > + > > + std::sort(sizes.begin(), sizes.end()); > > + > > + return sizes; > > +} > > > > /** > > * \brief Retrieve the camera sensor resolution > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp > > index 1bcd580e..8a40e955 100644 > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp > > @@ -64,16 +64,26 @@ std::vector<PixelFormat> CIO2Device::formats() const > > * \brief Retrieve the list of supported size ranges > > * \return The list of supported SizeRange > > */ > > -std::vector<SizeRange> CIO2Device::sizes() const > > +std::vector<SizeRange> CIO2Device::sizes(const PixelFormat &format) const > > I know we don't really doxygen, but as it's there, what about > documenting it ? > > > { > > + std::vector<SizeRange> szRange; > > Why have you moved it up ? I would also keep the same name > > > + int mbusCode = -1; > > + > > if (!sensor_) > > return {}; > > > > - std::vector<SizeRange> sizes; > > - for (const Size &size : sensor_->sizes()) > > - sizes.emplace_back(size, size); > > + for (const auto &iter : mbusCodesToPixelFormat) { > > + if (iter.second == format) > > + mbusCode = iter.first; > > There should be only one match, right ? Then > if (iter.second != format) > continue; > > mbusCode = iter.first; > break; > > > + } > > + > > + if (!mbusCode) > > if (-1) evaluates to true > > With this fixed: > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > + return {}; > > + > > + for (const Size &sz: sensor_->sizes(mbusCode)) checkstyle reports: - for (const Size &sz: sensor_->sizes(mbusCode)) + for (const Size &sz : sensor_->sizes(mbusCode)) szRange.emplace_back(sz); > > + szRange.emplace_back(sz); > > > > - return sizes; > > + return szRange; > > } > > > > /** > > diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h > > index f28e9f1d..24272dc5 100644 > > --- a/src/libcamera/pipeline/ipu3/cio2.h > > +++ b/src/libcamera/pipeline/ipu3/cio2.h > > @@ -35,7 +35,7 @@ public: > > CIO2Device(); > > > > std::vector<PixelFormat> formats() const; > > - std::vector<SizeRange> sizes() const; > > + std::vector<SizeRange> sizes(const PixelFormat &format) const; > > > > int init(const MediaDevice *media, unsigned int index); > > int configure(const Size &size, V4L2DeviceFormat *outputFormat); > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index 19cb5c4e..c73540fb 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -455,7 +455,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, > > bufferCount = cio2Config.bufferCount; > > > > for (const PixelFormat &format : data->cio2_.formats()) > > - streamFormats[format] = data->cio2_.sizes(); > > + streamFormats[format] = data->cio2_.sizes(format); > > > > break; > > } > > diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp > > index a8dcad82..372ee4af 100644 > > --- a/test/camera-sensor.cpp > > +++ b/test/camera-sensor.cpp > > @@ -76,7 +76,7 @@ protected: > > return TestFail; > > } > > > > - const std::vector<Size> &sizes = sensor_->sizes(); > > + const std::vector<Size> &sizes = sensor_->sizes(*iter); > > auto iter2 = std::find(sizes.begin(), sizes.end(), > > Size(4096, 2160)); > > if (iter2 == sizes.end()) { > > -- > > 2.31.1 > >
Hi Umang, On 25/08/2021 14:01, Jacopo Mondi wrote: > Hi Umang, > > one additional comment > > On Wed, Aug 25, 2021 at 12:09:35PM +0200, Jacopo Mondi wrote: >> Hi Umang, >> >> On Tue, Aug 10, 2021 at 01:28:51PM +0530, Umang Jain wrote: >>> In CameraSensor, the mbusCodes() and sizes() accessor functions >>> retrieves all the supported media bus codes and the supported sizes >>> respectively. However, this is quite limiting since the caller >>> probably isn't in a position to match which range of sizes are >>> supported for a particular mbusCode. >>> >>> Hence, the caller is most likely interested to know about the sizes >>> supported for a particular media bus code. This patch transforms the >>> existing CameraSensor::sizes() to CameraSensor::sizes(mbuscode) to >>> achieve that goal. >>> >>> The patch also transforms existing CIO2Device::sizes() in IPU3 pipeline >>> handler to CIO2Device::sizes(PixelFormat) on a similar principle. The >>> function is then plumbed to CameraSensor::sizes(mbusCode) to enumerate >>> the per-format sizes as required in >>> PipelineHandlerIPU3::generateConfiguration(). >>> >>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >>> Tested-by: Umang Jain <umang.jain@ideasonboard.com> >> >> Well, I hope you test your patches before sending them. Do we need >> Tested-by tags from the author ? >> >>> Tested-by: Jacopo Mondi <jacopo@jmondi.org> >> >> I was sure I had reviewed the series in full I'm sorry, >> >> >>> --- >>> include/libcamera/internal/camera_sensor.h | 2 +- >>> src/libcamera/camera_sensor.cpp | 25 ++++++++++++++++------ >>> src/libcamera/pipeline/ipu3/cio2.cpp | 20 ++++++++++++----- >>> src/libcamera/pipeline/ipu3/cio2.h | 2 +- >>> src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- >>> test/camera-sensor.cpp | 2 +- >>> 6 files changed, 38 insertions(+), 15 deletions(-) >>> >>> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h >>> index db12b07e..d25a1165 100644 >>> --- a/include/libcamera/internal/camera_sensor.h >>> +++ b/include/libcamera/internal/camera_sensor.h >>> @@ -38,7 +38,7 @@ public: >>> const std::string &id() const { return id_; } >>> const MediaEntity *entity() const { return entity_; } >>> const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } >>> - const std::vector<Size> &sizes() const { return sizes_; } >>> + const std::vector<Size> sizes(unsigned int mbusCode) const; >>> Size resolution() const; >>> const std::vector<int32_t> &testPatternModes() const >>> { >>> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp >>> index 7a386415..87668509 100644 >>> --- a/src/libcamera/camera_sensor.cpp >>> +++ b/src/libcamera/camera_sensor.cpp >>> @@ -472,14 +472,27 @@ int CameraSensor::initProperties() >>> */ >>> >>> /** >>> - * \fn CameraSensor::sizes() >>> - * \brief Retrieve the frame sizes supported by the camera sensor >>> + * \brief Retrieve the supported frame sizes for a media bus code >>> + * \param[in] mbusCode The media bus code for which sizes are requested >>> * >>> - * The reported sizes span all media bus codes supported by the camera sensor. >>> - * Not all sizes may be supported by all media bus codes. >>> - * The fact that this statement is removed makes this patch sound like a good idea! >>> - * \return The supported frame sizes sorted in increasing order >>> + * \return The supported frame sizes for \a mbusCode sorted in increasing order >>> */ >>> +const std::vector<Size> CameraSensor::sizes(unsigned int mbusCode) const >>> +{ >>> + std::vector<Size> sizes; >>> + >>> + const auto &format = formats_.find(mbusCode); >>> + if (format == formats_.end()) >>> + return sizes; >>> + >>> + const std::vector<SizeRange> &ranges = format->second; >>> + std::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes), >>> + [](const SizeRange &range) { return range.max; }); >>> + >>> + std::sort(sizes.begin(), sizes.end()); >>> + >>> + return sizes; >>> +} >>> >>> /** >>> * \brief Retrieve the camera sensor resolution >>> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp >>> index 1bcd580e..8a40e955 100644 >>> --- a/src/libcamera/pipeline/ipu3/cio2.cpp >>> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp >>> @@ -64,16 +64,26 @@ std::vector<PixelFormat> CIO2Device::formats() const >>> * \brief Retrieve the list of supported size ranges >>> * \return The list of supported SizeRange >>> */ >>> -std::vector<SizeRange> CIO2Device::sizes() const >>> +std::vector<SizeRange> CIO2Device::sizes(const PixelFormat &format) const >> >> I know we don't really doxygen, but as it's there, what about >> documenting it ? Agreed, we should keep that doc consistent. >> >>> { >>> + std::vector<SizeRange> szRange; >> >> Why have you moved it up ? I would also keep the same name >> >>> + int mbusCode = -1; >>> + is 0 a valid mbusCode? If not, we could use that and keep mbusCode as unsigned. https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/subdev-formats.html#v4l2-mbus-pixelcode shows a table of valid mbus codes, and I don't think its likely that 0x0 would ever be added there. >>> if (!sensor_) >>> return {}; >>> >>> - std::vector<SizeRange> sizes; >>> - for (const Size &size : sensor_->sizes()) >>> - sizes.emplace_back(size, size); >>> + for (const auto &iter : mbusCodesToPixelFormat) { >>> + if (iter.second == format) >>> + mbusCode = iter.first; >> >> There should be only one match, right ? Then >> if (iter.second != format) >> continue; >> >> mbusCode = iter.first; >> break; >> >>> + } >>> + >>> + if (!mbusCode) >> >> if (-1) evaluates to true Then this wouldn't be a problem... >> >> With this fixed: likewise Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> >> >>> + return {}; >>> + >>> + for (const Size &sz: sensor_->sizes(mbusCode)) > > checkstyle reports: > > - for (const Size &sz: sensor_->sizes(mbusCode)) > + for (const Size &sz : sensor_->sizes(mbusCode)) > szRange.emplace_back(sz); > >>> + szRange.emplace_back(sz); >>> >>> - return sizes; >>> + return szRange; >>> } >>> >>> /** >>> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h >>> index f28e9f1d..24272dc5 100644 >>> --- a/src/libcamera/pipeline/ipu3/cio2.h >>> +++ b/src/libcamera/pipeline/ipu3/cio2.h >>> @@ -35,7 +35,7 @@ public: >>> CIO2Device(); >>> >>> std::vector<PixelFormat> formats() const; >>> - std::vector<SizeRange> sizes() const; >>> + std::vector<SizeRange> sizes(const PixelFormat &format) const; >>> >>> int init(const MediaDevice *media, unsigned int index); >>> int configure(const Size &size, V4L2DeviceFormat *outputFormat); >>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp >>> index 19cb5c4e..c73540fb 100644 >>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp >>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp >>> @@ -455,7 +455,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, >>> bufferCount = cio2Config.bufferCount; >>> >>> for (const PixelFormat &format : data->cio2_.formats()) >>> - streamFormats[format] = data->cio2_.sizes(); >>> + streamFormats[format] = data->cio2_.sizes(format); >>> >>> break; >>> } >>> diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp >>> index a8dcad82..372ee4af 100644 >>> --- a/test/camera-sensor.cpp >>> +++ b/test/camera-sensor.cpp >>> @@ -76,7 +76,7 @@ protected: >>> return TestFail; >>> } >>> >>> - const std::vector<Size> &sizes = sensor_->sizes(); >>> + const std::vector<Size> &sizes = sensor_->sizes(*iter); >>> auto iter2 = std::find(sizes.begin(), sizes.end(), >>> Size(4096, 2160)); >>> if (iter2 == sizes.end()) { >>> -- >>> 2.31.1 >>>
diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index db12b07e..d25a1165 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -38,7 +38,7 @@ public: const std::string &id() const { return id_; } const MediaEntity *entity() const { return entity_; } const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } - const std::vector<Size> &sizes() const { return sizes_; } + const std::vector<Size> sizes(unsigned int mbusCode) const; Size resolution() const; const std::vector<int32_t> &testPatternModes() const { diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 7a386415..87668509 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -472,14 +472,27 @@ int CameraSensor::initProperties() */ /** - * \fn CameraSensor::sizes() - * \brief Retrieve the frame sizes supported by the camera sensor + * \brief Retrieve the supported frame sizes for a media bus code + * \param[in] mbusCode The media bus code for which sizes are requested * - * The reported sizes span all media bus codes supported by the camera sensor. - * Not all sizes may be supported by all media bus codes. - * - * \return The supported frame sizes sorted in increasing order + * \return The supported frame sizes for \a mbusCode sorted in increasing order */ +const std::vector<Size> CameraSensor::sizes(unsigned int mbusCode) const +{ + std::vector<Size> sizes; + + const auto &format = formats_.find(mbusCode); + if (format == formats_.end()) + return sizes; + + const std::vector<SizeRange> &ranges = format->second; + std::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes), + [](const SizeRange &range) { return range.max; }); + + std::sort(sizes.begin(), sizes.end()); + + return sizes; +} /** * \brief Retrieve the camera sensor resolution diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index 1bcd580e..8a40e955 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -64,16 +64,26 @@ std::vector<PixelFormat> CIO2Device::formats() const * \brief Retrieve the list of supported size ranges * \return The list of supported SizeRange */ -std::vector<SizeRange> CIO2Device::sizes() const +std::vector<SizeRange> CIO2Device::sizes(const PixelFormat &format) const { + std::vector<SizeRange> szRange; + int mbusCode = -1; + if (!sensor_) return {}; - std::vector<SizeRange> sizes; - for (const Size &size : sensor_->sizes()) - sizes.emplace_back(size, size); + for (const auto &iter : mbusCodesToPixelFormat) { + if (iter.second == format) + mbusCode = iter.first; + } + + if (!mbusCode) + return {}; + + for (const Size &sz: sensor_->sizes(mbusCode)) + szRange.emplace_back(sz); - return sizes; + return szRange; } /** diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h index f28e9f1d..24272dc5 100644 --- a/src/libcamera/pipeline/ipu3/cio2.h +++ b/src/libcamera/pipeline/ipu3/cio2.h @@ -35,7 +35,7 @@ public: CIO2Device(); std::vector<PixelFormat> formats() const; - std::vector<SizeRange> sizes() const; + std::vector<SizeRange> sizes(const PixelFormat &format) const; int init(const MediaDevice *media, unsigned int index); int configure(const Size &size, V4L2DeviceFormat *outputFormat); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 19cb5c4e..c73540fb 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -455,7 +455,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, bufferCount = cio2Config.bufferCount; for (const PixelFormat &format : data->cio2_.formats()) - streamFormats[format] = data->cio2_.sizes(); + streamFormats[format] = data->cio2_.sizes(format); break; } diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp index a8dcad82..372ee4af 100644 --- a/test/camera-sensor.cpp +++ b/test/camera-sensor.cpp @@ -76,7 +76,7 @@ protected: return TestFail; } - const std::vector<Size> &sizes = sensor_->sizes(); + const std::vector<Size> &sizes = sensor_->sizes(*iter); auto iter2 = std::find(sizes.begin(), sizes.end(), Size(4096, 2160)); if (iter2 == sizes.end()) {