<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p>Hi Jacopo<br>
    </p>
    <div class="moz-cite-prefix">On 8/9/21 9:19 PM, Jacopo Mondi wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:20210809154942.xkhgj4p2tqttz7oh@uno.localdomain">
      <pre class="moz-quote-pre" wrap="">Hi Umang,

On Tue, Aug 03, 2021 at 07:02:02PM +0530, Umang Jain wrote:
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">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 <a class="moz-txt-link-freetext" href="CameraSensor::sizes()">CameraSensor::sizes()</a> to <a class="moz-txt-link-freetext" href="CameraSensor::sizes(mbuscode)">CameraSensor::sizes(mbuscode)</a> to
achieve that goal.

To know all the frame sizes of the CameraSensor as required in IPU3
case <a class="moz-txt-link-freetext" href="Cio2Device::sizes()">Cio2Device::sizes()</a>, one would now require to enumerate all the
media bus codes (can be retrieved by <a class="moz-txt-link-freetext" href="CameraSensor::mbusCodes())">CameraSensor::mbusCodes())</a> with
<a class="moz-txt-link-freetext" href="CameraSensor::size(mbusCode)">CameraSensor::size(mbusCode)</a>. The result can be inserted in a
<a class="moz-txt-link-freetext" href="std::set">std::set</a><> to avoid duplicates.

Signed-off-by: Umang Jain <a class="moz-txt-link-rfc2396E" href="mailto:umang.jain@ideasonboard.com"><umang.jain@ideasonboard.com></a>
---
 include/libcamera/internal/camera_sensor.h |  2 +-
 src/libcamera/camera_sensor.cpp            | 36 ++++++++++++++++------
 src/libcamera/pipeline/ipu3/cio2.cpp       | 10 +++---
 test/camera-sensor.cpp                     |  2 +-
 4 files changed, 34 insertions(+), 16 deletions(-)

</pre>
      </blockquote>
    </blockquote>
    <pre class="moz-quote-pre" wrap=""><snip></pre>
    <blockquote type="cite"
      cite="mid:20210809154942.xkhgj4p2tqttz7oh@uno.localdomain">
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">
</pre>
      </blockquote>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">+                  allSizes.insert(sz);
+       }

-       return sizes;
+       return <a class="moz-txt-link-freetext" href="std::vector">std::vector</a><SizeRange>(allSizes.begin(), allSizes.end());
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
Looking at how <a class="moz-txt-link-freetext" href="CIO2::sizes()">CIO2::sizes()</a> is used in <a class="moz-txt-link-freetext" href="IPU3::generateConfiguration()">IPU3::generateConfiguration()</a>
I wonder if we shouldn't pass the desired PixelFormat to</pre>
    </blockquote>
    <p>I think you meant s/shouldn't/should here? So essentially do you
      want <br>
    </p>
    <blockquote>
      <p><font face="monospace"><a class="moz-txt-link-freetext" href="std::vector">std::vector</a><SizeRange>
          <a class="moz-txt-link-freetext" href="CIO2Device::sizes(const">CIO2Device::sizes(const</a> PixelFormat &format) const</font></p>
      <p><font face="monospace">?</font><br>
      </p>
    </blockquote>
    <p>(which looks okay to me)<br>
    </p>
    <blockquote type="cite"
      cite="mid:20210809154942.xkhgj4p2tqttz7oh@uno.localdomain">
      <pre class="moz-quote-pre" wrap="">
<a class="moz-txt-link-freetext" href="CIO2::sizes()">CIO2::sizes()</a> so that we can actually enumerate the per-format sizes
for real (and avoid going through the set->vector conversion).

</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap=""> }

 /**
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 <a class="moz-txt-link-freetext" href="std::vector">std::vector</a><Size> &sizes = sensor_->sizes();
+               const <a class="moz-txt-link-freetext" href="std::vector">std::vector</a><Size> &sizes = sensor_->sizes(*iter);
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
This changes the semantic of the test from "let's check if a format
supports 4096x2160" to "let's check if ARGB8888_1X32 supports
4096x2160". I think it's better now, and as long as the test passes,
since it uses VIMC, we should be good!</pre>
    </blockquote>
    I'll check! <br>
    <blockquote type="cite"
      cite="mid:20210809154942.xkhgj4p2tqttz7oh@uno.localdomain">
      <pre class="moz-quote-pre" wrap="">

Thanks
   j

</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">           auto iter2 = <a class="moz-txt-link-freetext" href="std::find(sizes.begin()">std::find(sizes.begin()</a>, sizes.end(),
                                       Size(4096, 2160));
                if (iter2 == sizes.end()) {
--
2.31.0

</pre>
      </blockquote>
    </blockquote>
  </body>
</html>