[libcamera-devel] [PATCH v4] py: cam.py: Provide live graph of request metadata

[Daniel Oakley]

Evening - thanks for reviewing this so far Tomi :)

On 07/03/2023 08:55, Tomi Valkeinen wrote:
> On 07/03/2023 08:56, Daniel Oakley wrote:
>> Hello,
>>
>> On 06/03/2023 14:26, Tomi Valkeinen wrote:
>>> Hi,
>>>
>>> On 02/02/2023 17:03, Daniel Oakley wrote:
>>>> Metadata is very useful when improving specific camera configurations.
>>>> Currently, there is an argument to display the metadata in text form,
>>>> however this can be hard to visualise and spot changes or patterns over
>>>> time. Therefore this proposed patch adds an argument to display this
>>>> metadata in graph form.
>>>>
>>>> The metadata graph has 3 optional parameters:
>>>>    - refresh, number of times a second to update the graph
>>>>    - buffer, amount of historic/previous data to show
>>>>    - graphs, number of graphs to split the metadata between
>>>>    - autoscale, whether or not to autoscale the axis so all the data fits
>>>>
>>>> Displaying the graph does have some performance penalty, however this
>>>> has been mostly mitigated through the refresh parameter. Despite this,
>>>> graphing might not the best of ideas when using the camera to record or
>>>> save data. This is mainly for debugging purposes.
>>>>
>>>> Suggested-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>>> Signed-off-by: Daniel Oakley <daniel.oakley at ideasonboard.com>
>>>> ---
>>>> This is the 4th version of my graph-metadata patch.
>>>> v3: hopefully addressing the issue of there being a lot of additional
>>>> code in the main cam.py which is optional. Most of the code has been
>>>> moved into a cam_metadata_graph.py file, which is imported at the start
>>>> of cam.py (as opposed to importing it twice throughout the code to make
>>>> the GraphDrawer and process the arguments). I have slightly tweaked the
>>>> error handling with the metadata-graph's arguments to catch a division
>>>> by zero error for refresh=0 and added a comment explanation for the
>>>> metadata-graph argument processor.
>>>> v4: I forgot to add the copywrite information to the new file, hopefully
>>>> I did it roughly correct.
>>>> There should, again, be no functional change between V1 and V2 and v3
>>>>
>>>>    src/py/cam/cam.py                |  22 +++
>>>>    src/py/cam/cam_metadata_graph.py | 236 +++++++++++++++++++++++++++++++
>>>>    2 files changed, 258 insertions(+)
>>>>    create mode 100644 src/py/cam/cam_metadata_graph.py
>>>
>>> What HW and OS did you use to test this?
>>>
>>> I tried with RPi OS, and after installing lots of stuff and finally getting libcamera compiling on the device, I got:
>>>
>>> Traceback (most recent call last):
>>>     File "/home/tomba/nfs/libcamera/./src/py/cam/cam.py", line 253, in event_handler
>>>       self.__request_handler(ctx, req)
>>>     File "/home/tomba/nfs/libcamera/./src/py/cam/cam.py", line 284, in __request_handler
>>>       ctx.graph_drawer.update_graph(ts, req.metadata)
>>>     File "/home/tomba/nfs/libcamera/src/py/cam/cam_metadata_graph.py", line 81, in update_graph
>>>       self.__animate()
>>>     File "/home/tomba/nfs/libcamera/src/py/cam/cam_metadata_graph.py", line 169, in __animate
>>>       self.blit_manager.update()
>>>     File "/home/tomba/nfs/libcamera/src/py/cam/cam_metadata_graph.py", line 206, in update
>>>       self.on_draw(None)
>>>     File "/home/tomba/nfs/libcamera/src/py/cam/cam_metadata_graph.py", line 189, in on_draw
>>>       self._draw_animated()
>>>     File "/home/tomba/nfs/libcamera/src/py/cam/cam_metadata_graph.py", line 200, in _draw_animated
>>>       fig.draw_artist(a)
>>>     File "/usr/lib/python3/dist-packages/matplotlib/figure.py", line 1879, in draw_artist
>>>       raise AttributeError("draw_artist can only be used after an "
>>> AttributeError: draw_artist can only be used after an initial draw which caches the renderer
>>>
>>> I'm still trying to get pyqt and matplotlib working on my buildroot...
>>>
>>
>> That's odd, I am quite sure it worked fine for me with matplotlib installed with
>> pip (I think) and latest libcamera compiled with the following commands running
>> on a RPI4 with RpiOS with the new Offical PiCam3:
>>
>> `meson setup -Dpycamera=enabled build` (configure the build)
>>
>> `ninja -C build; PYTHONPATH=./build/src/py/ python ./src/py/cam/cam.py -c1 -C --metadata-graph` (build and run)
>>
>> I just tested again and it seemed fine with my laptop (running arch), I will
>> setup my RPI4 again to test with a fresh install of everything.
>>
>> For details on my laptop if that helps:
>> - Python 3.10.9
>> - libcamera v0.0.4 commit (6cf637eb253a68edebe59505bea55435fafb00cd)
>> - matplotlib 3.6.2 from pip (--user install)
>>
>> I have just tried pulling and merging the latest libcamera (no conflicts) and
>> again it seems fine on my laptop. Nothing is actually plotted with my laptop
>> camera because there is no data except for time (which is ignored) - but it
>> doesn't stop the graph from updating etc.
>>
>> If there is anything I can do to help get it working for you (I wasn't expecting
>> any trouble), let me know - but in the meantime I will attempt to get it working
>> in an RPI4 environment.
> 
> I finally got it working. I have to say getting matplotlib working wasn't as easy as I thought...
> 
> The matplotlib I got via apt was 3.3.4, so probably your code just requires a more recent one. After installing matplotlib with pip, I had to hunt for a missing binary library (libcblas.so.3, which I found from libatlas3-base).
> 
> I noticed two issues:
> 
> The value labels on the Y axis do not change. So if, e.g. the Lux values are initially around 1000, I get a few labels around 1000. If the Lux then drops to 100, the labels stay.
> 

I am surprised I missed this tbh - but I know why. It was late in the testing
phases when I added the option for autoscaling, after Kieran mentioned manual
zoom not being as useful. I originally had manual zooming on the y-axis (I had 
disabled automatic for most of testing because I thought it was better) which
happened to updated the scales when you changed things. It was added it as an
option because practically, manual and automatic zoom are mutually exclusive.

In short, automatic zoom wasn't tested that much as it was re-added later due to
feedback - must have missed it in later testing. That is what reviews are for :)

Fixing this is another matter. I am in the middle of mock exams right now so free
time is scarce. I will get back with a fix however it won't be this week. I would
prefer it if you carried on reviewing despite of this - but it is up to you of
course (and the time you have available to spend on this)

For context, the axis does not update because "blit" - which is used to increase
performance - only updates certain parts of the graph.

> The second issue is perhaps not about this patch as such, but rather how cam.py handles frames. If I run:
> 
> cam.py -c 1 -s pixelformat=XRGB8888 -C -Rqt --metadata-graph
> 

I had not even considered this being a problem, partially because I get an error
running -Rqt with my cam3 so I always used a different backend or none at all.
I will respond further in your follow up email. Well done on catching this.

> I get prints like this:
> 
> 2053.301575 (30.00 fps) cam1-stream0: seq 33, bytes 1920000, CRCs []
> 2053.601499 (10.00 fps) cam1-stream0: seq 38, bytes 1920000, CRCs []
> 2053.501522 (5.00 fps) cam1-stream0: seq 35, bytes 1920000, CRCs []
> 2053.534850 (-15.00 fps) cam1-stream0: seq 36, bytes 1920000, CRCs []
> 2053.568177 (30.01 fps) cam1-stream0: seq 37, bytes 1920000, CRCs []
> 
> which then results in pretty interesting metadata graphs =). So we seem to be getting frames in wrong sequence. For some reason I can't reproduce this without --metadata-graph, by adding sleeps. I'll look at this further.
> 
>>> While I don't yet quite know how this behaves and looks like, I do wonder if this
>>> is something that would be better done with a separate script. We already have
>>> --save-frames option, we could add --save-metadata. And use matplotlib to show that
>>> data, possibly on a separate device (I'm thinking of showing it on my PC, as the dirs
>>> are shared via NFS).
>>
>> While I am sure this is possible, it would be a completely recode to get it
>> working in that way, and it would feel like more of a hack maintained (or not)
>> alongside libcamera. This is way more convenient to get setup (in theory
>> apparently). Kieran touch on this in a bit more detail in further replies I am
>> sure you have seen.
>>
>> For a remote solution, Kieran, while testing, managed to get X11 forwarding
>> working for matplotlib - however I am not sure if that specifically worked
>> outside of the box. I was running mine over VNC because I didn't have a display
>> on hand for my raspberry pi.
> 
> Yes, I can see that something like this is nice. I do wonder if matplotlib is a good choice for rapidly updating graphs, although it seems to work surprisingly well.

It took quite a bit of work to get it working so well on a raspberry pi 4 - but
it certainly work quite well. Matplotlib really great on a PC, but hardly any
metadata is available so its not very useful there. Due to the Pi, I had to add
in things like a custom refresh rate and blit to get performance where it needs
to be. Quite pleased with the result though.

On the topic of refresh rates, have you looked that the --metadata-graph options
yet? They are quite nifty ;)

> 
>  Tomi
> 

Thanks again for reviewing this - this have been a really valuable experience
for me,

Daniel