<div dir="ltr"><div dir="ltr">Hi David, <div><br></div><div>Thank you for your patch.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 30 Nov 2021 at 11:23, David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">When a raw stream is specified, the bit depth and packing requested<br>
should influence our choice of camera mode to match (if possible).<br>
<br>
Signed-off-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
---<br>
.../pipeline/raspberrypi/raspberrypi.cpp | 33 ++++++++++---------<br>
1 file changed, 18 insertions(+), 15 deletions(-)<br>
<br>
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
index 045725dd..03012e89 100644<br>
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
@@ -49,6 +49,8 @@ LOG_DEFINE_CATEGORY(RPI)<br>
<br>
namespace {<br>
<br>
+unsigned int DEFAULT_RAW_BIT_DEPTH = 12;<br>
+<br></blockquote><div><br></div><div>Perhaps a const, or constexpr for this one?</div><div>I would switch to camelCase as well.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
/* Map of mbus codes to supported sizes reported by the sensor. */<br>
using SensorFormats = std::map<unsigned int, std::vector<Size>>;<br>
<br>
@@ -125,15 +127,13 @@ double scoreFormat(double desired, double actual)<br>
return score;<br>
}<br>
<br>
-V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req)<br>
+V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req, unsigned int bitDepth)<br>
{<br>
double bestScore = std::numeric_limits<double>::max(), score;<br>
V4L2SubdeviceFormat bestFormat;<br>
<br>
#define PENALTY_AR 1500.0<br>
-#define PENALTY_8BIT 2000.0<br>
-#define PENALTY_10BIT 1000.0<br>
-#define PENALTY_12BIT 0.0<br>
+#define PENALTY_BIT_DEPTH 500.0<br></blockquote><div><br></div><div>Should we just switch to constexpr as well and do away with the #define?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
/* Calculate the closest/best mode from the user requested size. */<br>
for (const auto &iter : formatsMap) {<br>
@@ -152,12 +152,7 @@ V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &<br>
score += PENALTY_AR * scoreFormat(reqAr, fmtAr);<br>
<br>
/* Add any penalties... this is not an exact science! */<br>
- if (info.bitsPerPixel == 12)<br>
- score += PENALTY_12BIT;<br>
- else if (info.bitsPerPixel == 10)<br>
- score += PENALTY_10BIT;<br>
- else if (info.bitsPerPixel == 8)<br>
- score += PENALTY_8BIT;<br>
+ score += abs(info.bitsPerPixel - bitDepth) * PENALTY_BIT_DEPTH;<br></blockquote><div><br></div><div>Should we use the scoreFormat() call for this? That way, we can weight the scoring to a higher bit depth if an</div><div>exact match is not found. We may need to reword scoreFormat() appropriately.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
if (score <= bestScore) {<br>
bestScore = score;<br>
@@ -397,9 +392,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()<br>
* Calculate the best sensor mode we can use based on<br>
* the user request.<br>
*/<br>
- V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size);<br>
+ const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);<br>
+ unsigned int bitDepth = info.isValid() ? info.bitsPerPixel : DEFAULT_RAW_BIT_DEPTH;<br>
+ V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size, bitDepth);<br>
+ BayerFormat::Packing packing = BayerFormat::Packing::CSI2;<br>
+ if (info.isValid() && !info.packed)<br>
+ packing = BayerFormat::Packing::None;<br>
V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,<br>
- BayerFormat::Packing::CSI2);<br>
+ packing);<br>
int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);<br>
if (ret)<br>
return Invalid;<br>
@@ -533,7 +533,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,<br>
switch (role) {<br>
case StreamRole::Raw:<br>
size = data->sensor_->resolution();<br>
- sensorFormat = findBestFormat(data->sensorFormats_, size);<br>
+ sensorFormat = findBestFormat(data->sensorFormats_, size, DEFAULT_RAW_BIT_DEPTH);<br>
pixelFormat = mbusCodeToPixelFormat(sensorFormat.mbus_code,<br>
BayerFormat::Packing::CSI2);<br>
ASSERT(pixelFormat.isValid());<br>
@@ -622,6 +622,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)<br>
Size maxSize, sensorSize;<br>
unsigned int maxIndex = 0;<br>
bool rawStream = false;<br>
+ unsigned int bitDepth = DEFAULT_RAW_BIT_DEPTH;<br>
<br>
/*<br>
* Look for the RAW stream (if given) size as well as the largest<br>
@@ -638,7 +639,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)<br>
sensorSize = cfg.size;<br>
rawStream = true;<br> <br>
- packing = BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;<br>
+ BayerFormat bayerFormat = BayerFormat::fromPixelFormat(cfg.pixelFormat);<br>
+ packing = bayerFormat.packing;<br>
+ bitDepth = bayerFormat.bitDepth;<br>
} else {<br>
if (cfg.size > maxSize) {<br>
maxSize = config->at(i).size;<br>
@@ -664,7 +667,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)<br>
}<br>
<br>
/* First calculate the best sensor mode we can use based on the user request. */<br>
- V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize);<br>
+ V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize, bitDepth);<br>
ret = data->sensor_->setFormat(&sensorFormat);<br>
if (ret)<br>
return ret;<br></blockquote><div><br></div><div>With or without the suggestions:</div><div><br></div><div>Reviewed-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
-- <br>
2.30.2<br>
<br>
</blockquote></div></div>