<div dir="ltr"><div dir="ltr">Hi Paul,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, May 10, 2021 at 4:49 PM <<a href="mailto:paul.elder@ideasonboard.com">paul.elder@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 Hiro and Laurent,<br>
<br>
On Fri, May 07, 2021 at 03:06:36PM +0300, Laurent Pinchart wrote:<br>
> Hi Hiro,<br>
> <br>
> On Fri, May 07, 2021 at 11:55:47AM +0900, Hirokazu Honda wrote:<br>
<br>
<snip><br>
<br>
> > Overall, I am fine with this change while I am a bit concerned about the<br>
> > performance of allocate_camera_metadata and append_camera_metadata.<br>
> > I expect it causes O(N), where N is the data size, and in the worst case<br>
> > adding entries will be O(N*T), where T is the number of added entires.<br>
> > The data size is not considerably small (about 500 bytes), isn't it?<br>
> > Perhaps is it possible to cache the added entries in the CameraMetadata<br>
> > class until get() and form as camera_metadata_t upon get(), so adding<br>
> > entries will be O(N), where N is the data size.<br>
> <br>
> I've been thinking about it too, and I think it would make sense. We<br>
> could even ditch the Android metadata helper library completly and use a<br>
> custom implementation that would integrate better with our usage of the<br>
> metadata. I'd consider this as an improvement on top of this patch<br>
> though.<br>
<br>
I've given this some more thought, and I think the design that I started<br>
out with is probably better.<br>
<br>
We need to remember that we'll be producing a metadata on every result,<br>
so it does need to be quick. With what I have right now, it's mostly<br>
constant time insertion, and constant time get(). Some insertions might<br>
be linear (O(N+T)), but I think that can be mitigated by<br>
instrumentation, and then simply allocate slightly more to begin with.<br>
<br>
An alternative is to have guaranteed constant time insertion (either via<br>
vector or deque with some custom metadata entry). The issue with this is<br>
that we also have guaranteed linear time get(), as we have to walk the<br>
list to convert the libcamera metadata to android metadata. A mitigation<br>
for this would be to convert during insertion, but then we would be<br>
having to deal with android metadata manually and bypassing the android<br>
metadata library functions that are provided, and I think that's really<br>
not a good idea.<br>
<br>
Unless I misunderstood your (Hiro's) proposal. Instead of resizing, we<br>
create a new android metadata container and insert into there? We would<br>
still need to merge it in the end anyway, so I don't think the runtime<br>
would change compared to what I have here, it's just a matter of if the<br>
merging is during insertion or during get().<br>
<br></blockquote><div><br></div><div>I see your point. You're right.</div><div>I thought to simply cache the value with std::map.</div><div>But sadly a given value can be any type. We may have to create a heap storage for them or use std::variant.</div><div>It may complicate CameraMeatdata implementation.</div><div>I think your solution is better now.</div><div>  </div><div>-Hiro</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanks,<br>
<br>
Paul<br>
</blockquote></div></div>