[libcamera-devel] [RFC PATCH] android: camera_metadata: Auto-resize CameraMetadata

[Hirokazu Honda]

Hi Paul,

On Mon, May 10, 2021 at 4:49 PM <paul.elder at ideasonboard.com> wrote:

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

-Hiro

>
> Thanks,
>
> Paul
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210510/88114972/attachment.htm>