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

18238 diff mbox series

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@ideasonboard.com>
Signed-off-by: Daniel Oakley <daniel.oakley@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

Hey, sorry for bumping, but I would love for this to get a proper code
review. It would be nice to get this code into cam.py so people can
start using it if they need it :)

Thanks,
Daniel Oakley <daniel.oakley@ideasonboard.com>

On 02/02/2023 15: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@ideasonboard.com>
> Signed-off-by: Daniel Oakley <daniel.oakley@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
> 
> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py
> index 967a72f5..e5f6910a 100755
> --- a/src/py/cam/cam.py
> +++ b/src/py/cam/cam.py
> @@ -6,6 +6,7 @@
>   from typing import Any
>   import argparse
>   import binascii
> +import cam_metadata_graph as metadata_graph
>   import libcamera as libcam
>   import libcamera.utils
>   import sys
> @@ -21,6 +22,7 @@ class CameraContext:
>       opt_strict_formats: bool
>       opt_crc: bool
>       opt_metadata: bool
> +    opt_metadata_graph: dict
>       opt_save_frames: bool
>       opt_capture: int
>   
> @@ -39,6 +41,7 @@ class CameraContext:
>           self.id = 'cam' + str(idx)
>           self.reqs_queued = 0
>           self.reqs_completed = 0
> +        self.graph_drawer = None
>   
>       def do_cmd_list_props(self):
>           print('Properties for', self.id)
> @@ -171,6 +174,9 @@ class CameraContext:
>               self.stream_names[stream] = 'stream' + str(idx)
>               print('{}-{}: stream config {}'.format(self.id, self.stream_names[stream], stream.configuration))
>   
> +    def initialize_graph_drawer(self, graph_arguments):
> +        self.graph_drawer = metadata_graph.GraphDrawer(graph_arguments, self.camera.id)
> +
>       def alloc_buffers(self):
>           allocator = libcam.FrameBufferAllocator(self.camera)
>   
> @@ -271,6 +277,12 @@ class CaptureState:
>               for ctrl, val in reqmeta.items():
>                   print(f'\t{ctrl} = {val}')
>   
> +        if ctx.opt_metadata_graph:
> +            if not ctx.graph_drawer:
> +                ctx.initialize_graph_drawer(ctx.opt_metadata_graph)
> +
> +            ctx.graph_drawer.update_graph(ts, req.metadata)
> +
>           for stream, fb in buffers.items():
>               stream_name = ctx.stream_names[stream]
>   
> @@ -393,6 +405,7 @@ def main():
>       parser.add_argument('--crc', nargs=0, type=bool, action=CustomAction, help='Print CRC32 for captured frames')
>       parser.add_argument('--save-frames', nargs=0, type=bool, action=CustomAction, help='Save captured frames to files')
>       parser.add_argument('--metadata', nargs=0, type=bool, action=CustomAction, help='Print the metadata for completed requests')
> +    parser.add_argument('--metadata-graph', nargs='*', type=str, const=True, action=CustomAction, help='Live graph of metadata. Default args: graphs=3 buffer=100 autoscale=true refresh=10')
>       parser.add_argument('--strict-formats', type=bool, nargs=0, action=CustomAction, help='Do not allow requested stream format(s) to be adjusted')
>       parser.add_argument('-s', '--stream', nargs='+', action=CustomAction)
>       args = parser.parse_args()
> @@ -418,6 +431,15 @@ def main():
>           ctx.opt_metadata = args.metadata.get(cam_idx, False)
>           ctx.opt_strict_formats = args.strict_formats.get(cam_idx, False)
>           ctx.opt_stream = args.stream.get(cam_idx, ['role=viewfinder'])
> +
> +        ctx.opt_metadata_graph = args.metadata_graph.get(cam_idx, False)
> +        if ctx.opt_metadata_graph is not False:
> +            ctx.opt_metadata_graph = metadata_graph.process_args(ctx.opt_metadata_graph)
> +            # Invalid argument parameters return False
> +            if ctx.opt_metadata_graph is False:
> +                print('Invalid --metadata_graph arguments, see --help')
> +                sys.exit(-1)
> +
>           contexts.append(ctx)
>   
>       for ctx in contexts:
> diff --git a/src/py/cam/cam_metadata_graph.py b/src/py/cam/cam_metadata_graph.py
> new file mode 100644
> index 00000000..7992c0c7
> --- /dev/null
> +++ b/src/py/cam/cam_metadata_graph.py
> @@ -0,0 +1,236 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (C) 2023, Daniel Oakley <daniel.oakley@ideasonboard.com>
> +
> +import libcamera.utils
> +
> +
> +# GraphDrawer is a class that manages drawing the graph. It sets up the graph;
> +# manages graph settings; holds the metadata buffer (previous + current values);
> +# and updates the graph.
> +# We store physical lines for each metadata as their x (time) and y (data) can
> +# be directly updated instead of redrawing each axis - improves performance.
> +class GraphDrawer:
> +    graph_number: int
> +    buffer_size: int
> +    autoscale_axis: bool
> +    refresh_time: float
> +
> +    BLOCKED_TYPES = [list, tuple, libcamera._libcamera.Rectangle]
> +
> +    def __init__(self, graph_arguments, camera_name):
> +        # We only import matplotlib here to reduce mandatory dependencies
> +        import matplotlib.pyplot as plt
> +        self.plt = plt
> +
> +        self.graph_number = graph_arguments["graphs"]
> +        self.buffer_size = graph_arguments["buffer"]
> +        self.autoscale_axis = graph_arguments["autoscale"]
> +        # Convert FPS into a duration in nanoseconds
> +        self.refresh_time = 1 / graph_arguments["refresh"] * 10**9
> +        self.previous_time = 0
> +
> +        self.figure = plt.figure(camera_name)
> +        self.axis = []
> +        self.axis_group = {}
> +        self.metadata_buffer = {}
> +        self.lines = {}
> +        self.blit_manager = self.BlitManager(self.figure.canvas)
> +
> +        # Define the margins for drawing the graphs
> +        self.figure.subplots_adjust(
> +            top=0.95,
> +            bottom=0.05,
> +            left=0.1,
> +            right=0.95,
> +            hspace=0.2,
> +            wspace=0.2
> +        )
> +
> +        self.plt.show(block=False)
> +
> +    def update_graph(self, time, metadata):
> +        # On the first request, allocate the each metadata entry to an axis
> +        if len(self.axis) == 0:
> +            self.__divide_subplots(self.graph_number, metadata)
> +            self.plt.draw()
> +
> +        for ctrl, val in metadata.items():
> +            if type(val) in self.BLOCKED_TYPES or ctrl.name in ["SensorTimestamp"]:
> +                continue
> +
> +            # The metadata_buffer holds an x array and a y array for each ctrl
> +            # where x is the time series and y is the metadata value
> +            if not self.metadata_buffer.get(ctrl.name, False):
> +                self.metadata_buffer[ctrl.name] = [[], []]
> +
> +            # Create the lines and configure their style on the graph
> +            if not self.lines.get(ctrl.name, False):
> +                self.lines[ctrl.name], = self.axis_group[ctrl].plot([], label=ctrl.name)
> +                self.blit_manager.add_artist(self.lines[ctrl.name])
> +                self.lines[ctrl.name].set_alpha(0.7)
> +                self.lines[ctrl.name].set_linewidth(2.4)
> +
> +            self.metadata_buffer[ctrl.name][0].append(time)
> +            self.metadata_buffer[ctrl.name][1].append(val)
> +            # Remove the oldest entry to keep the buffer fixed at its max size
> +            if len(self.metadata_buffer[ctrl.name][0]) > self.buffer_size:
> +                del self.metadata_buffer[ctrl.name][0][0]
> +                del self.metadata_buffer[ctrl.name][1][0]
> +
> +        if time - self.previous_time >= self.refresh_time:
> +            self.__animate()
> +            self.previous_time = time
> +
> +    # This method allocates the metadata into the correct number of graphs based
> +    # on how different their example metadata is from their consecutive neighbour
> +    def __divide_subplots(self, number_of_graphs, example_metadata):
> +        # Create the correct number of axis positioned in a vertical stack
> +        for i in range(1, number_of_graphs + 1):
> +            axis = self.plt.subplot(number_of_graphs, 1, i)
> +            # Remove the visible x axis as it is not redrawn with blit
> +            axis.get_xaxis().set_ticks([])
> +            self.axis.append(axis)
> +
> +        cleansed_metadata = {}
> +        for ctrl, val in example_metadata.items():
> +            if not (type(val) in self.BLOCKED_TYPES or ctrl.name in ["SensorTimestamp"]):
> +                cleansed_metadata[ctrl] = val
> +
> +        # Summery of what the following code does:
> +        # We first sort the metadata items by value so we can identify the
> +        # difference between them.
> +        # From there we can split them up based on the differences between them.
> +        # We do this by sorting the the ctrls by their differences, and then
> +        # adding N breaks into the standardly sorted list of ctrls (by values)
> +        # next to those name. Where N is the number of graphs we want.
> +        # We then go through the ctrls in order, adding them to the correct
> +        # axis group (increasing their group if a break is found)
> +
> +        # Sort the metadata lowest to highest so consecutive values can be compared
> +        sorted_metadata = dict(sorted(cleansed_metadata.items(),
> +                               key=lambda item: item[1]))
> +
> +        # Create the dictionary containing the {ctrl:percentage difference}
> +        percent_diff = {}
> +        prev_val = None
> +        for ctrl,val in sorted_metadata.items():
> +            if prev_val:
> +                percent_diff[ctrl] = val/prev_val
> +
> +            prev_val = val
> +
> +        # Sort those percentage differences, highest to lowest, so we can find
> +        # the appropriate break points to separate the metadata by
> +        sorted_diffs = dict(sorted(percent_diff.items(),
> +                            key=lambda item: item[1], reverse=True))
> +        # This the list of ctrls ordered by value, lowest to highest
> +        sorted_ctrl = sorted(sorted_metadata, key=sorted_metadata.get)
> +
> +        # Add the correct number breaks in, starting with the break that
> +        # separates the greatest distance
> +        i = 0
> +        for ctrl, _ in sorted_diffs.items():
> +            i += 1
> +            if i >= number_of_graphs:
> +                break
> +
> +            sorted_ctrl.insert(sorted_ctrl.index(ctrl), "~BREAK~")
> +
> +        # Put the ctrls with their correct axis group, incrementing group when
> +        # a break is found.
> +        group = 0
> +        for ctrl in sorted_ctrl:
> +            if ctrl == "~BREAK~":
> +                group += 1
> +            else:
> +                self.axis_group[ctrl] = self.axis[group]
> +
> +    def __animate(self):
> +        for ctrl, series in self.metadata_buffer.items():
> +            self.lines[ctrl].set_xdata(series[0])
> +            self.lines[ctrl].set_ydata(series[1])
> +
> +        # Scale and display the legend on the axis
> +        for axis in self.axis:
> +            axis.relim()
> +            axis.legend(loc="upper left")
> +            axis.autoscale_view()
> +
> +            # Adjust the y scale to be bigger once if manual scaling
> +            if axis.get_autoscaley_on() and not self.autoscale_axis:
> +                axis_ymin, axis_ymax = axis.get_ybound()
> +                axis_yrange = axis_ymax - axis_ymin
> +                axis.set_ybound([axis_ymin - axis_yrange * 0.25,
> +                                axis_ymax + axis_yrange * 0.25])
> +
> +            axis.set_autoscaley_on(self.autoscale_axis)
> +            axis.set_autoscalex_on(True)
> +
> +        self.blit_manager.update()
> +
> +    # This BlitManager is derived from: (comments removed and init simplified)
> +    # matplotlib.org/devdocs/tutorials/advanced/blitting.html#class-based-example
> +    # The BlitManager manages redrawing the graph in a performant way. It also
> +    # prompts a full re-draw (axis and legend) whenever the window is resized.
> +    class BlitManager:
> +        def __init__(self, canvas):
> +            self.canvas = canvas
> +            self._bg = None
> +            self._artists = []
> +
> +            self.cid = canvas.mpl_connect("draw_event", self.on_draw)
> +
> +        def on_draw(self, event):
> +            cv = self.canvas
> +            if event is not None:
> +                if event.canvas != cv:
> +                    raise RuntimeError
> +            self._bg = cv.copy_from_bbox(cv.figure.bbox)
> +            self._draw_animated()
> +
> +        def add_artist(self, art):
> +            if art.figure != self.canvas.figure:
> +                raise RuntimeError
> +            art.set_animated(True)
> +            self._artists.append(art)
> +
> +        def _draw_animated(self):
> +            fig = self.canvas.figure
> +            for a in self._artists:
> +                fig.draw_artist(a)
> +
> +        def update(self):
> +            cv = self.canvas
> +            fig = cv.figure
> +            if self._bg is None:
> +                self.on_draw(None)
> +            else:
> +                cv.restore_region(self._bg)
> +                self._draw_animated()
> +                cv.blit(fig.bbox)
> +            cv.flush_events()
> +
> +
> +# This method processes the --metadata_graph arguments into a dictionary. If
> +# an error occurs while doing that (not valid data type or 0 value for refresh)
> +# false will be returned and the main script will exit with an error.
> +# Non-existing arguments are ignored.
> +def process_args(arguments):
> +    mdg_args = []
> +    for i in arguments:
> +        mdg_args += i.split("=")
> +    try:
> +        args_dict = {
> +            "graphs":  (3 if "graphs" not in mdg_args else
> +                       int(mdg_args[mdg_args.index("graphs") + 1])),
> +            "buffer":  (100 if "buffer" not in mdg_args else
> +                       int(mdg_args[mdg_args.index("buffer") + 1])),
> +            "refresh": (10.0 if "refresh" not in mdg_args else
> +                       1/1/float(mdg_args[mdg_args.index("refresh") + 1])),
> +            "autoscale": (True if "autoscale" not in mdg_args else
> +                (mdg_args[mdg_args.index("autoscale") + 1]).lower() == "true")
> +        }
> +        return args_dict
> +
> +    except (ValueError, ZeroDivisionError):
> +        return False

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@ideasonboard.com>
> Signed-off-by: Daniel Oakley <daniel.oakley@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...

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).

  Tomi

Quoting Tomi Valkeinen (2023-03-06 14:26:37)
> 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@ideasonboard.com>
> > Signed-off-by: Daniel Oakley <daniel.oakley@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...
> 
> 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).

But that doesn't work 'live'. I've tested this (or a previous version) -
and it allows me to get an instant and direct view of the repsonses in
the IPA algorithms as I adjust for lighting conditions etc.

--
Kieran


> 
>   Tomi
>

On 06/03/2023 16:59, Kieran Bingham wrote:
> Quoting Tomi Valkeinen (2023-03-06 14:26:37)
>> 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@ideasonboard.com>
>>> Signed-off-by: Daniel Oakley <daniel.oakley@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...
>>
>> 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).
> 
> But that doesn't work 'live'. I've tested this (or a previous version) -
> and it allows me to get an instant and direct view of the repsonses in
> the IPA algorithms as I adjust for lighting conditions etc.

Oh, I was thinking of "streaming" from the file. Although I'm not sure 
how bit lag there is for that with NFS. You could, of course, also run 
that locally, which I think would remove the lag fully.

  Tomi

Quoting Tomi Valkeinen (2023-03-06 15:02:04)
> On 06/03/2023 16:59, Kieran Bingham wrote:
> > Quoting Tomi Valkeinen (2023-03-06 14:26:37)
> >> 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@ideasonboard.com>
> >>> Signed-off-by: Daniel Oakley <daniel.oakley@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...
> >>
> >> 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).
> > 
> > But that doesn't work 'live'. I've tested this (or a previous version) -
> > and it allows me to get an instant and direct view of the repsonses in
> > the IPA algorithms as I adjust for lighting conditions etc.
> 
> Oh, I was thinking of "streaming" from the file. Although I'm not sure 
> how bit lag there is for that with NFS. You could, of course, also run 
> that locally, which I think would remove the lag fully.

If we're able to display 'live' from a file, then that would mean we
could have a single viewer that would work from stored / captured data,
cam, cam.py, and other applications. So I'm not opposed to that, I just
don't know how well it would run.

It also requires multi-step setup to view which is a bit more of a pain,
but anything we can do to make the implementation reusable is good.

--
Kieran


> 
>   Tomi
>

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@ideasonboard.com>
>> Signed-off-by: Daniel Oakley <daniel.oakley@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.

> 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.

>
>   Tomi
>
Thanks for attempting to review,
Daniel

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@ideasonboard.com>
>>> Signed-off-by: Daniel Oakley <daniel.oakley@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.

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 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.

  Tomi

On 07/03/2023 10: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@ideasonboard.com>
>>>> Signed-off-by: Daniel Oakley <daniel.oakley@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.
> 
> 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 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.

Ah, I think I have an idea about this.

If I use -Rqt, the cam.py's event loop is based on Qt's event loop and 
event handling. But matplotlib also uses Qt event loop (I think there 
are different backends, but that's probably what it uses here). And 
matplotlib probably uses synchronous waits, i.e. it forces the Qt event 
loop to run (again).

What happens is that first cam.py's event handler gets called by Qt. It 
starts processing the events, which leads to the metadata-graph code 
calling matplotlib's update code, which runs the Qt event loop, which 
causes the cam.py's event handler to get called. So, in the middle of 
processing Request N, we suddenly start handling Request N+1.

I think it's the cv.flush_events() call that does the sync update. 
Commenting that out makes Qt work. However, without flush_events(), when 
running without Qt rendering, no update is done...

  Tomi

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@ideasonboard.com>
>>>> Signed-off-by: Daniel Oakley <daniel.oakley@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