<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>