| Message ID | 20200902104730.43451-6-jacopo@jmondi.org |
|---|---|
| State | Superseded, archived |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, On 02/09/2020 11:47, Jacopo Mondi wrote: > The resolutions supported for the RAW formats cannot be tested from > a list of known sizes like the processed ones. This is mainly due to the > fact RAW streams are produced by capturing frames at the CSI-2 receiver > output and their size corresponds to the sensor's native sizes. > > In order to obtain the RAW frame size generate a temporary > CameraConfiguration for the Role::StillCaptureRAW role and inspect the > map of StreamFormats returned by the pipeline handler. Presumably, it will also be heavily dependant upon the actual chosen streams from the other requested streams ... and only a subset might really be available when a full configuration is used? > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/android/camera_device.cpp | 21 +++++++++++++++++---- > src/android/camera_device.h | 2 ++ > 2 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index 765c3292e4f3..181fca83988d 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -314,6 +314,17 @@ std::vector<Size> CameraDevice::listProcessedResolutions(CameraConfiguration *ca > return supportedResolutions; > } > > +std::vector<Size> CameraDevice::listRawResolutions(const libcamera::PixelFormat &pixelFormat) > +{ same comment on the function name, that we're filtering (rather than 'listing' valid resolutions. Though maybe this one is subtly different... but I'd keep both namings consistent either way. > + std::unique_ptr<CameraConfiguration> cameraConfig = > + camera_->generateConfiguration({ StillCaptureRaw }); > + StreamConfiguration &cfg = cameraConfig->at(0); > + const StreamFormats &formats = cfg.formats(); > + std::vector<Size> supportedResolutions = formats.sizes(pixelFormat); I'm a bit weary of this. If you apply this function to a UVC camera for example, it would simply return a list of sizes which represent the available frame sizes... Scratch that, I see it's filtered by pixelFormat ... ok - so I guess that works. I wonder if this couldn't also be handled by the other filter fucntion but I assume not easily, as you've done this ;-) Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + > + return supportedResolutions; > +} > + > /* > * Initialize the format conversion map to translate from Android format > * identifier to libcamera pixel formats and fill in the list of supported > @@ -466,13 +477,15 @@ int CameraDevice::initializeStreamConfigurations() > << camera3Format.name << " to: " > << mappedFormat.toString(); > > + std::vector<Size> resolutions; > const PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat); > if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) > - continue; > + resolutions = listRawResolutions(mappedFormat); > + else > + resolutions = listProcessedResolutions(cameraConfig.get(), > + mappedFormat, > + cameraResolutions); > > - std::vector<Size> resolutions = listProcessedResolutions(cameraConfig.get(), > - mappedFormat, > - cameraResolutions); > for (const Size &res : resolutions) { > streamConfigurations_.push_back({ res, androidFormat }); > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h > index 18fd5ff03cde..230e89b011e6 100644 > --- a/src/android/camera_device.h > +++ b/src/android/camera_device.h > @@ -97,6 +97,8 @@ private: > listProcessedResolutions(libcamera::CameraConfiguration *cameraConfig, > const libcamera::PixelFormat &pixelFormat, > const std::vector<libcamera::Size> &resolutions); > + std::vector<libcamera::Size> > + listRawResolutions(const libcamera::PixelFormat &pixelFormat); > > std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize(); > libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer); >
Hi Kieran, On Wed, Sep 02, 2020 at 02:15:44PM +0100, Kieran Bingham wrote: > Hi Jacopo, > > On 02/09/2020 11:47, Jacopo Mondi wrote: > > The resolutions supported for the RAW formats cannot be tested from > > a list of known sizes like the processed ones. This is mainly due to the > > fact RAW streams are produced by capturing frames at the CSI-2 receiver > > output and their size corresponds to the sensor's native sizes. > > > > In order to obtain the RAW frame size generate a temporary > > CameraConfiguration for the Role::StillCaptureRAW role and inspect the > > map of StreamFormats returned by the pipeline handler. > > Presumably, it will also be heavily dependant upon the actual chosen > streams from the other requested streams ... and only a subset might > really be available when a full configuration is used? Not sure I got you here :/ > > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/android/camera_device.cpp | 21 +++++++++++++++++---- > > src/android/camera_device.h | 2 ++ > > 2 files changed, 19 insertions(+), 4 deletions(-) > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > > index 765c3292e4f3..181fca83988d 100644 > > --- a/src/android/camera_device.cpp > > +++ b/src/android/camera_device.cpp > > @@ -314,6 +314,17 @@ std::vector<Size> CameraDevice::listProcessedResolutions(CameraConfiguration *ca > > return supportedResolutions; > > } > > > > +std::vector<Size> CameraDevice::listRawResolutions(const libcamera::PixelFormat &pixelFormat) > > +{ > > same comment on the function name, that we're filtering (rather than > 'listing' valid resolutions. Though maybe this one is subtly > different... but I'd keep both namings consistent either way. Well, we're actually listing them here :D And anyway, there's not much filtering going on here, maybe a bit more in the corresponding non-RAW implementation as we actually test a list of sizes and report only the supported ones. In any case, I would keep the naming of the two functions consistent whatever we chose. > > > + std::unique_ptr<CameraConfiguration> cameraConfig = > > + camera_->generateConfiguration({ StillCaptureRaw }); > > + StreamConfiguration &cfg = cameraConfig->at(0); > > + const StreamFormats &formats = cfg.formats(); > > + std::vector<Size> supportedResolutions = formats.sizes(pixelFormat); > > I'm a bit weary of this. > > If you apply this function to a UVC camera for example, it would simply > return a list of sizes which represent the available frame sizes... > Pardon my ignorance, does uvc support RAW ? > Scratch that, I see it's filtered by pixelFormat ... ok - so I guess > that works. > > > I wonder if this couldn't also be handled by the other filter fucntion > but I assume not easily, as you've done this ;-) Well, they do two different things. I actually considered doing what I'm doing here (building on top of StreamFormats) for non-RAW stream, but it turned out to be a whack-a-mole game of finding the right Role to request to have the sizes for the format we are interested in. And that gets pretty pipeline-implementation-dependent as there's no strict rule on what formats corresponds to a role. So I might be looking for supported NV12 sizes and my best bet is to ask for Viewfinder and see what StreamFormats are returned, but it's a bet and some pipeline handler might only support RGB for viewfinder and NV12 for StillCapture (it's an example, not sure it makes any sense) so I could end up testing all roles, filtering double results, re-ordering etc... Seems like a no-go to me. The other way around is possible as well: list RAW sizes by using the same method as it's used for non-RAW ones, with the difference that we accept adjusted sizes, as the pipeline handler will adjust to the closest available sensor frame size for each tested image resolution. Again, it seemd sub-optimal and requires filtering doubles and testing an un-necessary number of times, so I created two separate functions in the end. Be aware that in the back of my mind there's also a listJPEGSizes() that queries the encoder for the reasons explained in my other reply, if that makes any sense :) Thanks j > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > + > > + return supportedResolutions; > > +} > > + > > /* > > * Initialize the format conversion map to translate from Android format > > * identifier to libcamera pixel formats and fill in the list of supported > > @@ -466,13 +477,15 @@ int CameraDevice::initializeStreamConfigurations() > > << camera3Format.name << " to: " > > << mappedFormat.toString(); > > > > + std::vector<Size> resolutions; > > const PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat); > > if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) > > - continue; > > + resolutions = listRawResolutions(mappedFormat); > > + else > > + resolutions = listProcessedResolutions(cameraConfig.get(), > > + mappedFormat, > > + cameraResolutions); > > > > - std::vector<Size> resolutions = listProcessedResolutions(cameraConfig.get(), > > - mappedFormat, > > - cameraResolutions); > > for (const Size &res : resolutions) { > > streamConfigurations_.push_back({ res, androidFormat }); > > > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h > > index 18fd5ff03cde..230e89b011e6 100644 > > --- a/src/android/camera_device.h > > +++ b/src/android/camera_device.h > > @@ -97,6 +97,8 @@ private: > > listProcessedResolutions(libcamera::CameraConfiguration *cameraConfig, > > const libcamera::PixelFormat &pixelFormat, > > const std::vector<libcamera::Size> &resolutions); > > + std::vector<libcamera::Size> > > + listRawResolutions(const libcamera::PixelFormat &pixelFormat); > > > > std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize(); > > libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer); > > > > -- > Regards > -- > Kieran
On 02/09/2020 14:36, Jacopo Mondi wrote: > Hi Kieran, > > On Wed, Sep 02, 2020 at 02:15:44PM +0100, Kieran Bingham wrote: >> Hi Jacopo, >> >> On 02/09/2020 11:47, Jacopo Mondi wrote: >>> The resolutions supported for the RAW formats cannot be tested from >>> a list of known sizes like the processed ones. This is mainly due to the >>> fact RAW streams are produced by capturing frames at the CSI-2 receiver >>> output and their size corresponds to the sensor's native sizes. >>> >>> In order to obtain the RAW frame size generate a temporary >>> CameraConfiguration for the Role::StillCaptureRAW role and inspect the >>> map of StreamFormats returned by the pipeline handler. >> >> Presumably, it will also be heavily dependant upon the actual chosen >> streams from the other requested streams ... and only a subset might >> really be available when a full configuration is used? > > Not sure I got you here :/ I was worried about the fact that the pipeline handlers would configure the sensor in a specific format to handle the other streams, which might not match what android requests as the raw size ... but I don't think it would happen. Asking for a RAW stream should always be the 'size' of the main image I expect... (or the 'rawest' form thereof...) >> >> >> >>> >>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> >>> --- >>> src/android/camera_device.cpp | 21 +++++++++++++++++---- >>> src/android/camera_device.h | 2 ++ >>> 2 files changed, 19 insertions(+), 4 deletions(-) >>> >>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp >>> index 765c3292e4f3..181fca83988d 100644 >>> --- a/src/android/camera_device.cpp >>> +++ b/src/android/camera_device.cpp >>> @@ -314,6 +314,17 @@ std::vector<Size> CameraDevice::listProcessedResolutions(CameraConfiguration *ca >>> return supportedResolutions; >>> } >>> >>> +std::vector<Size> CameraDevice::listRawResolutions(const libcamera::PixelFormat &pixelFormat) >>> +{ >> >> same comment on the function name, that we're filtering (rather than >> 'listing' valid resolutions. Though maybe this one is subtly >> different... but I'd keep both namings consistent either way. > > Well, we're actually listing them here :D > And anyway, there's not much filtering going on here, maybe a bit more > in the corresponding non-RAW implementation as we actually test a list > of sizes and report only the supported ones. > > In any case, I would keep the naming of the two functions consistent > whatever we chose. > >> >>> + std::unique_ptr<CameraConfiguration> cameraConfig = >>> + camera_->generateConfiguration({ StillCaptureRaw }); >>> + StreamConfiguration &cfg = cameraConfig->at(0); >>> + const StreamFormats &formats = cfg.formats(); >>> + std::vector<Size> supportedResolutions = formats.sizes(pixelFormat); >> >> I'm a bit weary of this. >> >> If you apply this function to a UVC camera for example, it would simply >> return a list of sizes which represent the available frame sizes... >> > > Pardon my ignorance, does uvc support RAW ? probably not... >> Scratch that, I see it's filtered by pixelFormat ... ok - so I guess >> that works. Sorry - I left my working/thoughts in place. That sentence voided the previous one. >> >> >> I wonder if this couldn't also be handled by the other filter fucntion >> but I assume not easily, as you've done this ;-) > > Well, they do two different things. I actually considered doing what > I'm doing here (building on top of StreamFormats) for non-RAW stream, > but it turned out to be a whack-a-mole game of finding the right Role > to request to have the sizes for the format we are interested in. And > that gets pretty pipeline-implementation-dependent as there's no > strict rule on what formats corresponds to a role. So I might be > looking for supported NV12 sizes and my best bet is to ask for > Viewfinder and see what StreamFormats are returned, but it's a bet and > some pipeline handler might only support RGB for viewfinder and NV12 > for StillCapture (it's an example, not sure it makes any sense) so I > could end up testing all roles, filtering double results, re-ordering > etc... Seems like a no-go to me. > > The other way around is possible as well: list RAW sizes by using the > same method as it's used for non-RAW ones, with the difference that we > accept adjusted sizes, as the pipeline handler will adjust to the > closest available sensor frame size for each tested image resolution. > Again, it seemd sub-optimal and requires filtering doubles and testing > an un-necessary number of times, so I created two separate functions > in the end. > > Be aware that in the back of my mind there's also a listJPEGSizes() > that queries the encoder for the reasons explained in my other reply, > if that makes any sense :) Well, we'll see what happens when we get our next encoder ... > Thanks > j > > >> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >> >>> + >>> + return supportedResolutions; >>> +} >>> + >>> /* >>> * Initialize the format conversion map to translate from Android format >>> * identifier to libcamera pixel formats and fill in the list of supported >>> @@ -466,13 +477,15 @@ int CameraDevice::initializeStreamConfigurations() >>> << camera3Format.name << " to: " >>> << mappedFormat.toString(); >>> >>> + std::vector<Size> resolutions; >>> const PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat); >>> if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) >>> - continue; >>> + resolutions = listRawResolutions(mappedFormat); >>> + else >>> + resolutions = listProcessedResolutions(cameraConfig.get(), >>> + mappedFormat, >>> + cameraResolutions); >>> >>> - std::vector<Size> resolutions = listProcessedResolutions(cameraConfig.get(), >>> - mappedFormat, >>> - cameraResolutions); >>> for (const Size &res : resolutions) { >>> streamConfigurations_.push_back({ res, androidFormat }); >>> >>> diff --git a/src/android/camera_device.h b/src/android/camera_device.h >>> index 18fd5ff03cde..230e89b011e6 100644 >>> --- a/src/android/camera_device.h >>> +++ b/src/android/camera_device.h >>> @@ -97,6 +97,8 @@ private: >>> listProcessedResolutions(libcamera::CameraConfiguration *cameraConfig, >>> const libcamera::PixelFormat &pixelFormat, >>> const std::vector<libcamera::Size> &resolutions); >>> + std::vector<libcamera::Size> >>> + listRawResolutions(const libcamera::PixelFormat &pixelFormat); >>> >>> std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize(); >>> libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer); >>> >> >> -- >> Regards >> -- >> Kieran
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 765c3292e4f3..181fca83988d 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -314,6 +314,17 @@ std::vector<Size> CameraDevice::listProcessedResolutions(CameraConfiguration *ca return supportedResolutions; } +std::vector<Size> CameraDevice::listRawResolutions(const libcamera::PixelFormat &pixelFormat) +{ + std::unique_ptr<CameraConfiguration> cameraConfig = + camera_->generateConfiguration({ StillCaptureRaw }); + StreamConfiguration &cfg = cameraConfig->at(0); + const StreamFormats &formats = cfg.formats(); + std::vector<Size> supportedResolutions = formats.sizes(pixelFormat); + + return supportedResolutions; +} + /* * Initialize the format conversion map to translate from Android format * identifier to libcamera pixel formats and fill in the list of supported @@ -466,13 +477,15 @@ int CameraDevice::initializeStreamConfigurations() << camera3Format.name << " to: " << mappedFormat.toString(); + std::vector<Size> resolutions; const PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat); if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) - continue; + resolutions = listRawResolutions(mappedFormat); + else + resolutions = listProcessedResolutions(cameraConfig.get(), + mappedFormat, + cameraResolutions); - std::vector<Size> resolutions = listProcessedResolutions(cameraConfig.get(), - mappedFormat, - cameraResolutions); for (const Size &res : resolutions) { streamConfigurations_.push_back({ res, androidFormat }); diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 18fd5ff03cde..230e89b011e6 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -97,6 +97,8 @@ private: listProcessedResolutions(libcamera::CameraConfiguration *cameraConfig, const libcamera::PixelFormat &pixelFormat, const std::vector<libcamera::Size> &resolutions); + std::vector<libcamera::Size> + listRawResolutions(const libcamera::PixelFormat &pixelFormat); std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize(); libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);
The resolutions supported for the RAW formats cannot be tested from a list of known sizes like the processed ones. This is mainly due to the fact RAW streams are produced by capturing frames at the CSI-2 receiver output and their size corresponds to the sensor's native sizes. In order to obtain the RAW frame size generate a temporary CameraConfiguration for the Role::StillCaptureRAW role and inspect the map of StreamFormats returned by the pipeline handler. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/android/camera_device.cpp | 21 +++++++++++++++++---- src/android/camera_device.h | 2 ++ 2 files changed, 19 insertions(+), 4 deletions(-)