<div dir="ltr"><div dir="ltr">Hi Jacopo and Laurent,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Sep 19, 2024 at 5:09 PM Jacopo Mondi <<a href="mailto:jacopo.mondi@ideasonboard.com">jacopo.mondi@ideasonboard.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">Hi Harvey<br>
<br>
On Wed, Sep 18, 2024 at 04:33:27PM GMT, Cheng-Hao Yang wrote:<br>
> Sorry Jacopo, my bad to miss the first message:<br>
><br>
> On Mon, Sep 16, 2024 at 8:45 PM Jacopo Mondi <<a href="mailto:jacopo.mondi@ideasonboard.com" target="_blank">jacopo.mondi@ideasonboard.com</a>><br>
> wrote:<br>
><br>
> > Hi Harvey<br>
> ><br>
> > On Mon, Sep 16, 2024 at 03:51:00PM GMT, Cheng-Hao Yang wrote:<br>
> > > Hi Jacopo,<br>
> > ><br>
> > > On Mon, Sep 16, 2024 at 3:35 PM Jacopo Mondi <<br>
> > <a href="mailto:jacopo.mondi@ideasonboard.com" target="_blank">jacopo.mondi@ideasonboard.com</a>><br>
> > > wrote:<br>
> > ><br>
> > > > Hi Harvey<br>
> > > ><br>
> > > > On Mon, Sep 16, 2024 at 04:51:54AM GMT, Harvey Yang wrote:<br>
> > > > > Hi folks,<br>
> > > > ><br>
> > > > > Currently applications set resolutions, pixelFormat, bufferCount,<br>
> > etc,<br>
> > > > > into StreamConfigurations, and Pipeline Handler decides which streams<br>
> > > > > they're assigned to. However, it doesn't allow application to assign<br>
> > > > > streams that cannot be distinguished by those arguments into<br>
> > > > > VideoRecording or StillCapture (say YUV/NV12 format), which is<br>
> > needed in<br>
> > > > > mtkisp7.<br>
> > > ><br>
> > > > Could you explain in a bit more detail why this "is needed" and how<br>
> > > > you plan to use StreamRole as part of the stream configuration ?<br>
> > > ><br>
> ><br>
> > Could you explain in a bit more detail why this "is needed" and how<br>
> > you plan to use StreamRole as part of the stream configuration ?<br>
> ><br>
> ><br>
> ><br>
> In mtkisp7 pipeline handler, both preview (or video) and still capture<br>
> streams support the same format (NV12) and bufferCount, and the<br>
> maximum resolutions are also the same. Therefore, when calling<br>
> `validate()` and `configure()`, mtkisp7 pipeline handler doesn't know<br>
> if applications/Android adapter wants the stream to go through the<br>
> still captur pipeline or not.<br>
<br>
I still have an hard time understanding why validate() and configure()<br>
needs to know the "role". Is this to assign "pipes" to Streams ?</blockquote><div><br></div><div><div>If I understand correctly that "pipes" means the pipelines of ISP/IPA algos</div><div>for preview/video/still capture respectively, yes, it's to assign the</div><div>StreamConfiguration(s) to the desired pipe(s), which means to call</div><div>`StreamConfiguration::setStream()`. According to the comments [1], it's</div><div>supposed to be called in `PipelineHandler::configure()`, like</div><div>SimplePipelineHandler [2]. However, there are also some pipeline handlers</div><div>that set them in `CameraConfiguration::validate()` [3] [4].</div></div><div> </div><div><div>[1]: <a href="https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline_handler.cpp#n303">https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline_handler.cpp#n303</a></div><div>[2]: <a href="https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline/simple/simple.cpp#n1270">https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline/simple/simple.cpp#n1270</a></div><div>[3]: <a href="https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline/ipu3/ipu3.cpp#n347">https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline/ipu3/ipu3.cpp#n347</a></div><div>[4]: <a href="https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline/rkisp1/rkisp1.cpp#n520">https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline/rkisp1/rkisp1.cpp#n520</a></div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Does<br>
the mtkisp7 pipes have different capabilities when it comes to<br>
supported formats and resolutions ?<br></blockquote><div><br></div><div>No, as you can see in mtkisp7's implementation [5], all of them support the</div><div>same formats and resolutions.</div><div><br></div><div>[5]: <a href="https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/pipeline/mtkisp7/mtkisp7.cpp;l=556">https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/pipeline/mtkisp7/mtkisp7.cpp;l=556</a></div><div><br></div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
><br>
> Does that make sense :P?<br>
<br>
I guess so, but I wonder if the "role" is the main criteria that<br>
should be taken into account when assiging pipes to streams.<br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
How would this work for applications that operates on platforms where<br>
the pipe selection depends on other criteria like the supported image<br>
formats and stream resolutions ? In example, for rkisp1<br>
the "self" pipelines can do RGB while the "main" can't and that's how<br>
we assign pipes during validate(). </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
To be honest this has not proven to be optimal and I'm not opposed to<br>
add Roles to the pipe selection criteria, but they shouldn't be made the<br>
only criteria on which pipes are assigned (unless we support this on<br>
all pipelines).<br>
<br></blockquote><div><div><br class="gmail-Apple-interchange-newline">I agree that the `role` might not be the main criteria in general cases.</div><div>It's just that we find difficulties in assigning them properly with the current</div><div>configurations in mtkisp7.</div><div><br></div></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">
Also I would not based any future-proof design on Roles, they will<br>
likely be heavily reworked or removed going forward.<br>
<br>
As your main use case is Android, I think it would be doable for you<br>
to<br>
1) Request a generateConfiguration() and keep track of the<br>
   StreamConfiguration order.<br>
<br>
   generateConfiguration(Viewfinder, Still, Raw)<br>
<br>
   will give you StreamConfiguration for the above rols in order as<br>
   you have requested them, if supported.<br>
<br>
2) Code validate() so that you try to assing pipes based on the<br>
   formats/sizes and if formats/sizes are the same define an ordering.<br>
   In example the first YUV/WxH stream goes to Viewfinder if an<br>
   identical YUV/WxH stream is requested for Still Capture.<br></blockquote><div><br></div><div>This actually assumes the application doesn't change the content of the</div><div>default CameraConfiguration/StreamConfiguration, which I doubt if it's a</div><div>good thing. Currently in Android adapter, it re-arranges [6]</div><div>StreamConfigurations based on the previously-fetched default</div><div>StreamConfigurations of the StreamRoles, which I think is a normal</div><div>practice of the current interfaces' logic. Please correct me if I'm wrong :)</div><div><br></div><div>[6]: <a href="https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n708">https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n708</a><br></div><div><br></div><div>Also, if an application calls</div><div>`generateConfiguration(camera, {StreamRole::VideoRecording})` and</div><div>`generateConfiguration(camera, {StreamRole::StillCapture})`</div><div>respectively, and ends up calling `configure()` with `VideoRecording`'s</div><div>CameraConfiguration, how would the pipeline handler know that the</div><div>application intends to get buffers from the stream `VideoRecording`?</div><div>In your suggestion, it seems that the application needs to call</div><div>`generateConfiguration(camera, {StreamRole::VideoRecording})` again,</div><div>which I don't think is enforced in the current libcamera API, and might</div><div>not be a good practice.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
3) As validate() assigns Steams * to StreamConfiguration (with<br>
  ::setStream) it should be easy to keep track to which pipe a<br>
  StreamConfiguration has been assigned to at configure() time.<br>
<br>
Could you list what are the platform's pipes capabilities ?<br>
<br></blockquote><div><br></div><div>For example, on `ciri` with mtkisp7 pipeline handler [5], `pixelFormat` is NV12,</div><div>supported resolution range is `320x240` to `2592x1944`, and `bufferCount` is</div><div>8. It applies to `StillCapture`, `VideoRecording`, and `ViewFinder`.</div><div> </div><div>[5]: <a href="https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/pipeline/mtkisp7/mtkisp7.cpp;l=556">https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/pipeline/mtkisp7/mtkisp7.cpp;l=556</a><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
As said, I'm not opposed to use Roles for assigning pipes, but I don't<br>
think we should based any new development on Roles as there's an high<br>
chance they can be reworked.<br></blockquote><div><br></div><div>Could you briefly describe how the new APIs would be like?</div><div><br></div><div>Basically I think either applications need to have an argument to indicate</div><div>which kind of stream the StreamConfiguration needs to be assigned to, or</div><div>the pipeline handler needs to remember which StreamRole a</div><div>StreamConfiguration returned was asked as the default configuration for.</div><div>It doesn't have to be `StreamRole`. If there are other types/arguments in</div><div>the new reworked API, I'd be happy to adapt to the new one(s).</div><div><br></div><div>Two naive ideas:<br>1. Keep the new `StreamRole role;` to be a const member variable in</div><div>StreamConfiguration, which should be set in</div><div>`PipelineHandler::generateConfiguration()`.</div><div><br></div><div>2. If it makes sense, add a private class for `StreamConfiguration` and keep</div><div>the new `StreamRole role;` there, so that applications cannot get the info.</div><div>(I don't think we need to hide the info from the applications though...)</div><div><br></div><div>WDYT?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Cc-Laurent for opinions.<br>
<br>
><br>
> BR,<br>
> Harvey<br>
</blockquote></div><br clear="all"><div><br></div><span class="gmail_signature_prefix">-- </span><br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>BR,</div>Harvey Yang</div></div></div>