<div dir="ltr">Gentle ping: perfetto is an important feature in CrOS to do the debugging. It'd be nice to have it supported earlier.<div>Thanks!</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jul 30, 2024 at 3:30 PM Cheng-Hao Yang <<a href="mailto:chenghaoyang@chromium.org">chenghaoyang@chromium.org</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"><div dir="ltr"><div dir="ltr">Hi Paul,<div><br></div><div>Sorry for the late reply. I've been working on mtkisp7's migration :p</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Dec 27, 2022 at 12:09 PM Paul Elder <<a href="mailto:paul.elder@ideasonboard.com" target="_blank">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 Harvey,<br>
<br>
On Tue, Dec 20, 2022 at 07:05:23AM +0000, Harvey Yang via libcamera-devel wrote:<br>
> As ChromeOS is using perfetto (project named as CrOSetto). When ChromeOS<br>
> uses libcamera, it should use perfetto to collect traces instead of<br>
> lttng.<br>
<br>
I'll start with the elephant in the room: we cannot have duplicate<br>
tracepoint definitions. There must be only one definition of the<br>
tracepoints, and either one or both of perfetto's and lttng's<br>
tracepoints must be generated from that.<br>
<br>
For instance, I've already got a series pending [1] that expands on the<br>
request tracepoints quite a bit; it's clearly unscalable to deal with<br>
two duplicate sets of tracepoints (I'm not sure why I have to defend<br>
deduplication, but anyway just trying to be clear about my stance).<br>
<br>
[1] <a href="https://patchwork.libcamera.org/project/libcamera/list/?series=3668" rel="noreferrer" target="_blank">https://patchwork.libcamera.org/project/libcamera/list/?series=3668</a><br>
<br>
More on this in the relevant section in the patch.<br>
<br>
> <br>
> Signed-off-by: Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
> ---<br>
>  Documentation/guides/tracing.rst              | 125 ++++++++++++++----<br>
>  include/libcamera/internal/<a href="http://tracepoints.h.in" rel="noreferrer" target="_blank">tracepoints.h.in</a>   |  38 +++++-<br>
>  .../internal/tracepoints/meson.build          |  25 ++--<br>
>  .../internal/tracepoints/pipeline.perfetto    |  10 ++<br>
>  .../internal/tracepoints/request.perfetto     |  30 +++++<br>
>  meson.build                                   |   7 +-<br>
>  src/android/cros/camera3_hal.cpp              |   5 +<br>
>  src/android/cros/meson.build                  |   1 +<br>
>  src/libcamera/meson.build                     |  14 +-<br>
>  src/libcamera/perfetto/meson.build            |   6 +<br>
>  src/libcamera/perfetto/pipeline_perfetto.cpp  |  24 ++++<br>
>  src/libcamera/perfetto/request_perfetto.cpp   |  73 ++++++++++<br>
>  src/libcamera/tracepoints.cpp                 |  11 ++<br>
>  13 files changed, 325 insertions(+), 44 deletions(-)<br>
>  create mode 100644 include/libcamera/internal/tracepoints/pipeline.perfetto<br>
>  create mode 100644 include/libcamera/internal/tracepoints/request.perfetto<br>
>  create mode 100644 src/libcamera/perfetto/meson.build<br>
>  create mode 100644 src/libcamera/perfetto/pipeline_perfetto.cpp<br>
>  create mode 100644 src/libcamera/perfetto/request_perfetto.cpp<br>
> <br>
> diff --git a/Documentation/guides/tracing.rst b/Documentation/guides/tracing.rst<br>
> index ae960d85..bd5b3453 100644<br>
> --- a/Documentation/guides/tracing.rst<br>
> +++ b/Documentation/guides/tracing.rst<br>
> @@ -16,33 +16,50 @@ at periodic points in time. This can be done with other tools such as<br>
>  callgrind, perf, gprof, etc., without modification to the application,<br>
>  and is out of scope for this guide.<br>
>  <br>
> +Library: Perfetto vs lttng-ust<br>
> +------------------------------<br>
> +<br>
> +To integrate with CrOS tracing infrastructure, which uses perfetto (or called<br>
<br>
The first instance of "CrOS" should probably be spelled out.<br>
<br>
> +CrOSetto in CrOS), we implement the tracepoint macros (will be described below)<br>
> +with two different libraries: CrOS with perfetto, and the rest with lttng-ust.<br>
> +<br>
>  Compiling<br>
>  ---------<br>
>  <br>
> -To compile libcamera with tracing support, it must be enabled through the<br>
> -meson ``tracing`` option. It depends on the lttng-ust library (available in the<br>
> -``liblttng-ust-dev`` package for Debian-based distributions).<br>
> -By default the tracing option in meson is set to ``auto``, so if<br>
> -liblttng is detected, it will be enabled by default. Conversely, if the option<br>
> -is set to disabled, then libcamera will be compiled without tracing support.<br>
> +To compile libcamera with tracing support in CrOS, it must be enabled through<br>
<br>
s/ in CrOS//<br>
<br>
> +the meson ``tracing`` option. In CrOS, it depends on the perfetto library,<br>
> +which is by default available in CrOS; In the rest, it depends on the lttng-ust<br>
<br>
s/;/./<br>
<br>
s/the rest/non-CrOS/<br>
<br>
> +library (available in the ``liblttng-ust-dev`` package for Debian-based<br>
> +distributions).<br>
> +By default the tracing option in meson is set to ``auto``, so if the library is<br>
> +detected, it will be enabled by default. Conversely, if the option is set to<br>
> +disabled, then libcamera will be compiled without tracing support.<br>
>  <br>
>  Defining tracepoints<br>
>  --------------------<br>
>  <br>
>  libcamera already contains a set of tracepoints. To define additional<br>
> -tracepoints, create a file<br>
> -``include/libcamera/internal/tracepoints/{file}.tp``, where ``file`` is a<br>
> -reasonable name related to the category of tracepoints that you wish to<br>
> -define. For example, the tracepoints file for the Request object is called<br>
> -``request.tp``. An entry for this file must be added in<br>
> -``include/libcamera/internal/tracepoints/meson.build``.<br>
> -<br>
> -In this tracepoints file, define your tracepoints `as mandated by lttng<br>
> +tracepoints, create two files ``{file}.tp`` and ``{file}.perfetto``, for lttng<br>
> +and perfetto respectively, under ``include/libcamera/internal/tracepoints/``,<br>
> +and the perfetto implementation file ``src/libcamera/{file}_perfetto.cpp``,<br>
> +where ``file`` is a reasonable name related to the category of tracepoints that<br>
> +you wish to define. For example, the tracepoints files for the Request object is<br>
> +called ``request.tp`` and ``request.perfetto``. Entries for the files must be<br>
> +added in ``include/libcamera/internal/tracepoints/meson.build``.<br>
> +<br>
> +In the perfetto tracepoints files, declare the tracepoint functions in<br>
> +``{file}.perfetto``, and define them in ``{file}_perfetto.cpp``, `as mandated<br>
> +by perfetto <<a href="https://perfetto.dev/docs/instrumentation/track-events" rel="noreferrer" target="_blank">https://perfetto.dev/docs/instrumentation/track-events</a>>`_.<br>
> +Currently the only enabled perfetto::Category is ``libcamera``. If you intend to<br>
> +use another category, remember to define it in<br>
> +``include/libcamera/internal/<a href="http://tracepoints.h.in" rel="noreferrer" target="_blank">tracepoints.h.in</a>``, and enable it when collecting<br>
> +traces.<br>
<br>
These sections will have to be fixed.<br>
<br>
> +<br>
> +In the lttng tracepoints file, define your tracepoints `as mandated by lttng<br>
>  <<a href="https://lttng.org/man/3/lttng-ust" rel="noreferrer" target="_blank">https://lttng.org/man/3/lttng-ust</a>>`_. The header boilerplate must *not* be<br>
>  included (as it will conflict with the rest of our infrastructure), and<br>
>  only the tracepoint definitions (with the ``TRACEPOINT_*`` macros) should be<br>
>  included.<br>
> -<br>
>  All tracepoint providers shall be ``libcamera``. According to lttng, the<br>
>  tracepoint provider should be per-project; this is the rationale for this<br>
>  decision. To group tracepoint events, we recommend using<br>
> @@ -68,9 +85,9 @@ Then to use the tracepoint:<br>
>  <br>
>  ``LIBCAMERA_TRACEPOINT({tracepoint_event}, args...)``<br>
>  <br>
> -This macro must be used, as opposed to lttng's macros directly, because<br>
> -lttng is an optional dependency of libcamera, so the code must compile and run<br>
> -even when lttng is not present or when tracing is disabled.<br>
> +This macro must be used, as opposed to perfetto's or lttng's macros directly,<br>
> +because tracing support is optional of libcamera, so the code must compile and<br>
<br>
s/of/in/<br>
<br>
> +run even when perfetto or lttng are not present or when tracing is disabled.<br>
>  <br>
>  The tracepoint provider name, as declared in the tracepoint definition, is not<br>
>  included in the parameters of the tracepoint.<br>
> @@ -87,21 +104,75 @@ respectively. These are the tracepoints that our sample analysis script<br>
>  (see "Analyzing a trace") scans for when computing statistics on IPA call time.<br>
>  <br>
>  Using tracepoints (from an application)<br>
> ----------------------------------------<br>
> +---------------------------------------------<br>
>  <br>
>  As applications are not part of libcamera, but rather users of libcamera,<br>
>  applications should seek their own tracing mechanisms. For ease of tracing<br>
>  the application alongside tracing libcamera, it is recommended to also<br>
> -`use lttng <<a href="https://lttng.org/docs/#doc-tracing-your-own-user-application" rel="noreferrer" target="_blank">https://lttng.org/docs/#doc-tracing-your-own-user-application</a>>`_.<br>
> +`use lttng <<a href="https://lttng.org/docs/#doc-tracing-your-own-user-application" rel="noreferrer" target="_blank">https://lttng.org/docs/#doc-tracing-your-own-user-application</a>>`_,<br>
> +or `use perfetto <<a href="https://perfetto.dev/docs/instrumentation/tracing-sdk" rel="noreferrer" target="_blank">https://perfetto.dev/docs/instrumentation/tracing-sdk</a>>`_.<br>
>  <br>
>  Using tracepoints (from closed-source IPA)<br>
> -------------------------------------------<br>
> +------------------------------------------------<br>
<br>
Underline length.<br>
<br>
> +<br>
> +Similar to applications, closed-source IPAs can simply use perfetto or lttng<br>
> +on their own, or any other tracing mechanism if desired.<br>
> +<br>
> +Collecting a perfetto trace<br>
> +---------------------------<br>
> +<br>
> +A trace can be collected with the following steps:<br>
> +<br>
> +1. Start `traced` if it hasn't been started.<br>
> +.. code-block:: bash<br>
> +<br>
> +   start traced<br>
> +<br>
> +2. Start a consumer that includes “track_event” data source in the trace<br>
> +   config, and “libcamera” category.<br>
> +.. code-block:: bash<br>
> +<br>
> +   perfetto -c - --txt -o /tmp/perfetto-trace \<br>
<br>
(Reply to your comment in the other email) I think this is sufficient,<br>
as apparently the option for a config file is documented in the help<br>
text.<br>
<br>
> +   <<EOF<br>
> +<br>
> +   buffers: {<br>
> +       size_kb: 63488<br>
> +       fill_policy: DISCARD<br>
> +   }<br>
> +   buffers: {<br>
> +       size_kb: 2048<br>
> +       fill_policy: DISCARD<br>
> +   }<br>
> +   data_sources: {<br>
> +        config {<br>
> +            name: "track_event"<br>
> +            track_event_config {<br>
> +                enabled_categories: "libcamera"<br>
> +            }<br>
> +        }<br>
> +   }<br>
> +   duration_ms: 10000<br>
> +<br>
> +   EOF<br>
> +<br>
> +3. Execute the libcamera behavior you intend to trace during the set duration<br>
> +   (10000 ms) in the above example.<br>
> +<br>
> +4. After the consumer (cmd `perfetto`) is done, you can find the trace result<br>
> +   in ``/tmp/perfetto-trace``.<br>
> +<br>
> +Analyzing a perfetto trace<br>
> +--------------------------<br>
> +<br>
> +Follow the `guide <<a href="https://perfetto.dev/docs/visualization/perfetto-ui" rel="noreferrer" target="_blank">https://perfetto.dev/docs/visualization/perfetto-ui</a>>`_ to<br>
> +visualize the trace.<br>
>  <br>
> -Similar to applications, closed-source IPAs can simply use lttng on their own,<br>
> -or any other tracing mechanism if desired.<br>
> +In other words, upload the trace to `Perfetto UI <<a href="https://ui.perfetto.dev/" rel="noreferrer" target="_blank">https://ui.perfetto.dev/</a>>`_,<br>
> +where you can check the timeline of tracepoints on each process/thread. You can<br>
> +also run SQL queries to do analysis.<br>
>  <br>
> -Collecting a trace<br>
> -------------------<br>
> +Collecting an lttng trace<br>
> +------------------------<br>
<br>
Underline length.<br>
<br>
>  <br>
>  A trace can be collected fairly simply from lttng:<br>
>  <br>
> @@ -123,8 +194,8 @@ viewed by: ``lttng view -t $PATH_TO_TRACE``, where ``$PATH_TO_TRACE`` is the<br>
>  path that was printed when the session was created. This is the same path that<br>
>  is used when analyzing traces programatically, as described in the next section.<br>
>  <br>
> -Analyzing a trace<br>
> ------------------<br>
> +Analyzing a lttng trace<br>
<br>
s/a/an/<br>
<br>
> +-----------------------<br>
>  <br>
>  As mentioned above, while an lttng tracing session exists and the trace is not<br>
>  running, the trace output can be viewed as text by ``lttng view``.<br>
> diff --git a/include/libcamera/internal/<a href="http://tracepoints.h.in" rel="noreferrer" target="_blank">tracepoints.h.in</a> b/include/libcamera/internal/<a href="http://tracepoints.h.in" rel="noreferrer" target="_blank">tracepoints.h.in</a><br>
> index d0fc1365..b6c4abea 100644<br>
> --- a/include/libcamera/internal/<a href="http://tracepoints.h.in" rel="noreferrer" target="_blank">tracepoints.h.in</a><br>
> +++ b/include/libcamera/internal/<a href="http://tracepoints.h.in" rel="noreferrer" target="_blank">tracepoints.h.in</a><br>
> @@ -9,7 +9,24 @@<br>
>  #ifndef __LIBCAMERA_INTERNAL_TRACEPOINTS_H__<br>
>  #define __LIBCAMERA_INTERNAL_TRACEPOINTS_H__<br>
>  <br>
> -#if HAVE_TRACING<br>
> +#if HAVE_PERFETTO<br>
> +<br>
> +#include <perfetto/perfetto.h><br>
> +<br>
> +PERFETTO_DEFINE_CATEGORIES(<br>
> +     perfetto::Category("libcamera")<br>
> +             .SetDescription("Events from libcamera"));<br>
> +<br>
> +#define LIBCAMERA_TRACEPOINT(t_name, ...) \<br>
> +LIBCAMERA_TRACE_EVENT_##t_name(__VA_ARGS__)<br>
> +<br>
> +#define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func) \<br>
> +LIBCAMERA_TRACE_EVENT_ipa_call_begin(#pipe, #func)<br>
> +<br>
> +#define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func) \<br>
> +LIBCAMERA_TRACE_EVENT_ipa_call_end(#pipe, #func)<br>
> +<br>
> +#elif HAVE_LTTNG /* !HAVE_PERFETTO */<br>
>  #define LIBCAMERA_TRACEPOINT(...) tracepoint(libcamera, __VA_ARGS__)<br>
>  <br>
>  #define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func) \<br>
> @@ -18,7 +35,7 @@ tracepoint(libcamera, ipa_call_begin, #pipe, #func)<br>
>  #define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func) \<br>
>  tracepoint(libcamera, ipa_call_end, #pipe, #func)<br>
>  <br>
> -#else<br>
> +#else /* !HAVE_PERFETTO && !HAVE_LTTNG */<br>
>  <br>
>  namespace {<br>
>  <br>
> @@ -34,12 +51,17 @@ inline void unused([[maybe_unused]] Args&& ...args)<br>
>  #define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func)<br>
>  #define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func)<br>
>  <br>
> -#endif /* HAVE_TRACING */<br>
> +#endif /* HAVE_PERFETTO */<br>
>  <br>
>  #endif /* __LIBCAMERA_INTERNAL_TRACEPOINTS_H__ */<br>
>  <br>
> +#if HAVE_PERFETTO || HAVE_LTTNG<br>
> +<br>
> +#if HAVE_PERFETTO<br>
>  <br>
> -#if HAVE_TRACING<br>
> +#include <perfetto/perfetto.h><br>
> +<br>
> +#else /* HAVE_LTTNG */<br>
>  <br>
>  #undef TRACEPOINT_PROVIDER<br>
>  #define TRACEPOINT_PROVIDER libcamera<br>
> @@ -47,15 +69,21 @@ inline void unused([[maybe_unused]] Args&& ...args)<br>
>  #undef TRACEPOINT_INCLUDE<br>
>  #define TRACEPOINT_INCLUDE "{{path}}"<br>
>  <br>
> +#endif /* HAVE_PERFETTO */<br>
> +<br>
>  #if !defined(INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H) || defined(TRACEPOINT_HEADER_MULTI_READ)<br>
>  #define INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H<br>
<br>
I'm pretty sure #endif /* HAVE_PERFETTO */ should come here (unless<br>
perfetto also needs this? Does it?).<br>
<br>
>  <br>
> +#if HAVE_LTTNG<br>
>  #include <lttng/tracepoint.h><br>
> +#endif  /* HAVE_LTTNG */<br>
>  <br>
>  {{source}}<br>
>  <br>
>  #endif /* INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H */<br>
<br>
Same for this.<br>
<br>
I wonder if it's easier just to have a top-level `if HAVE_LTTNG and if<br>
HAVE_PERFETTO` and just duplicate {{source}}. It's only one line that's<br>
duplicated anyway.<br>
<br>
>  <br>
> +#if HAVE_LTTNG<br>
>  #include <lttng/tracepoint-event.h><br>
> +#endif  /* HAVE_LTTNG */<br>
>  <br>
> -#endif /* HAVE_TRACING */<br>
> +#endif /* HAVE_PERFETTO || HAVE_LTTNG */<br>
> diff --git a/include/libcamera/internal/tracepoints/meson.build b/include/libcamera/internal/tracepoints/meson.build<br>
> index d9b2fca5..ff5aece6 100644<br>
> --- a/include/libcamera/internal/tracepoints/meson.build<br>
> +++ b/include/libcamera/internal/tracepoints/meson.build<br>
> @@ -1,12 +1,19 @@<br>
>  # SPDX-License-Identifier: CC0-1.0<br>
>  <br>
> -# enum files must go first<br>
> -tracepoint_files = files([<br>
> -    'buffer_enums.tp',<br>
> -    'request_enums.tp',<br>
> -])<br>
> +if get_option('android').enabled() and get_option('android_platform') == 'cros'<br>
> +  tracepoint_files = files([<br>
> +      'pipeline.perfetto',<br>
> +      'request.perfetto',<br>
> +  ])<br>
> +else<br>
> +  # enum files must go first<br>
> +  tracepoint_files = files([<br>
> +      'buffer_enums.tp',<br>
> +      'request_enums.tp',<br>
> +  ])<br>
>  <br>
> -tracepoint_files += files([<br>
> -    'pipeline.tp',<br>
> -    'request.tp',<br>
> -])<br>
> +  tracepoint_files += files([<br>
> +      'pipeline.tp',<br>
> +      'request.tp',<br>
> +  ])<br>
> +endif<br>
<br>
And this is going to need a bit more moving parts for code generation...<br>
<br>
> diff --git a/include/libcamera/internal/tracepoints/pipeline.perfetto b/include/libcamera/internal/tracepoints/pipeline.perfetto<br>
> new file mode 100644<br>
> index 00000000..5f45295e<br>
> --- /dev/null<br>
> +++ b/include/libcamera/internal/tracepoints/pipeline.perfetto<br>
> @@ -0,0 +1,10 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2022, Google Inc.<br>
> + *<br>
> + * pipeline.tp - Tracepoints for pipelines<br>
> + */<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_ipa_call_begin(const char *pipe, const char *func);<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_ipa_call_end(const char *pipe, const char *func);<br>
<br>
Basically, we already have definitions for these tracepoints in<br>
include/libcamera/internal/tracepoints/pipeline.tp. It's obviously not a<br>
good idea to have duplicate definitions.<br>
<br>
So we either need to use lttng's tracepoints definitions to generate<br>
perfetto's here, or we create a new language from which we generate<br>
tracepoints for both. imo the latter is overkill (and costs a lot more<br>
effort) so probably the former is better.<br>
<br>
I'll use the request tracepoints as examples below just because they<br>
have a bit more beef than these pipeline tracepoints which are just two<br>
strings.<br>
<br>
> diff --git a/include/libcamera/internal/tracepoints/request.perfetto b/include/libcamera/internal/tracepoints/request.perfetto<br>
> new file mode 100644<br>
> index 00000000..fd6a42a4<br>
> --- /dev/null<br>
> +++ b/include/libcamera/internal/tracepoints/request.perfetto<br>
> @@ -0,0 +1,30 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2022, Google Inc.<br>
> + *<br>
> + * request.tp - Tracepoints for the request object<br>
> + */<br>
> +<br>
> +#include <libcamera/internal/request.h><br>
> +<br>
> +#include <libcamera/framebuffer.h><br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request(libcamera::Request *req);<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_construct(libcamera::Request *req);<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_destroy(libcamera::Request *req);<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_reuse(libcamera::Request *req);<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_queue(libcamera::Request *req);<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_device_queue(libcamera::Request *req);<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_complete(libcamera::Request::Private *req);<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_cancel(libcamera::Request::Private *req);<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_complete_buffer(<br>
> +             libcamera::Request::Private *req,<br>
> +             libcamera::FrameBuffer * buf);<br>
> diff --git a/meson.build b/meson.build<br>
> index e4c0be16..79514b5b 100644<br>
> --- a/meson.build<br>
> +++ b/meson.build<br>
> @@ -156,7 +156,11 @@ libcamera_includes = include_directories('include')<br>
>  py_modules = []<br>
>  <br>
>  # Libraries used by multiple components<br>
> -liblttng = dependency('lttng-ust', required : get_option('tracing'))<br>
> +libperfetto = dependency('perfetto', required : get_option('tracing').enabled()<br>
> +    and get_option('android').enabled()<br>
> +    and get_option('android_platform') == 'cros')<br>
> +liblttng = cc.find_library('lttng-ust', required : get_option('tracing').enabled()<br>
> +    and not libperfetto.found())<br>
>  <br>
>  # Pipeline handlers<br>
>  #<br>
> @@ -208,6 +212,7 @@ py_mod.find_installation('python3', modules: py_modules)<br>
>  summary({<br>
>              'Enabled pipelines': pipelines,<br>
>              'Enabled IPA modules': enabled_ipa_modules,<br>
> +            'Perfetto support': perfetto_enabled,<br>
>              'Tracing support': tracing_enabled,<br>
>              'Android support': android_enabled,<br>
>              'GStreamer support': gst_enabled,<br>
> diff --git a/src/android/cros/camera3_hal.cpp b/src/android/cros/camera3_hal.cpp<br>
> index fb863b5f..2fbd7f14 100644<br>
> --- a/src/android/cros/camera3_hal.cpp<br>
> +++ b/src/android/cros/camera3_hal.cpp<br>
> @@ -7,10 +7,15 @@<br>
>  <br>
>  #include <cros-camera/cros_camera_hal.h><br>
>  <br>
> +#include "libcamera/internal/tracepoints.h"<br>
>  #include "../camera_hal_manager.h"<br>
>  <br>
>  static void set_up([[maybe_unused]] cros::CameraMojoChannelManagerToken *token)<br>
>  {<br>
> +     perfetto::TracingInitArgs args;<br>
> +     args.backends |= perfetto::kSystemBackend;<br>
> +     perfetto::Tracing::Initialize(args);<br>
> +     perfetto::TrackEvent::Register();<br>
>  }<br>
>  <br>
>  static void tear_down()<br>
> diff --git a/src/android/cros/meson.build b/src/android/cros/meson.build<br>
> index 35995dd8..68f2bd9e 100644<br>
> --- a/src/android/cros/meson.build<br>
> +++ b/src/android/cros/meson.build<br>
> @@ -9,5 +9,6 @@ android_hal_sources += files([<br>
>  ])<br>
>  <br>
>  android_deps += dependency('libcros_camera')<br>
> +android_deps += dependency('perfetto')<br>
>  <br>
>  android_cpp_args += ['-DOS_CHROMEOS']<br>
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build<br>
> index 0494e808..ba8bf26e 100644<br>
> --- a/src/libcamera/meson.build<br>
> +++ b/src/libcamera/meson.build<br>
> @@ -62,6 +62,7 @@ libthreads = dependency('threads')<br>
>  <br>
>  subdir('base')<br>
>  subdir('ipa')<br>
> +subdir('perfetto')<br>
>  subdir('pipeline')<br>
>  subdir('proxy')<br>
>  <br>
> @@ -89,11 +90,19 @@ if not libcrypto.found()<br>
>      warning('Neither gnutls nor libcrypto found, all IPA modules will be isolated')<br>
>  endif<br>
>  <br>
> -if liblttng.found()<br>
> +if libperfetto.found()<br>
> +    perfetto_enabled = true<br>
> +    tracing_enabled = false<br>
> +    config_h.set('HAVE_PERFETTO', 1)<br>
> +    libcamera_sources += perfetto_sources<br>
> +    libcamera_sources += files(['tracepoints.cpp'])<br>
> +elif liblttng.found()<br>
> +    perfetto_enabled = false<br>
>      tracing_enabled = true<br>
> -    config_h.set('HAVE_TRACING', 1)<br>
> +    config_h.set('HAVE_LTTNG', 1)<br>
>      libcamera_sources += files(['tracepoints.cpp'])<br>
>  else<br>
> +    perfetto_enabled = false<br>
>      tracing_enabled = false<br>
>  endif<br>
>  <br>
> @@ -154,6 +163,7 @@ libcamera_deps = [<br>
>      libcrypto,<br>
>      libdl,<br>
>      liblttng,<br>
> +    libperfetto,<br>
>      libudev,<br>
>      libyaml,<br>
>  ]<br>
> diff --git a/src/libcamera/perfetto/meson.build b/src/libcamera/perfetto/meson.build<br>
> new file mode 100644<br>
> index 00000000..119a1ad7<br>
> --- /dev/null<br>
> +++ b/src/libcamera/perfetto/meson.build<br>
> @@ -0,0 +1,6 @@<br>
> +# SPDX-License-Identifier: CC0-1.0<br>
> +<br>
> +perfetto_sources = files([<br>
> +    'pipeline_perfetto.cpp',<br>
> +    'request_perfetto.cpp',<br>
> +])<br>
> diff --git a/src/libcamera/perfetto/pipeline_perfetto.cpp b/src/libcamera/perfetto/pipeline_perfetto.cpp<br>
> new file mode 100644<br>
> index 00000000..07b82ffd<br>
> --- /dev/null<br>
> +++ b/src/libcamera/perfetto/pipeline_perfetto.cpp<br>
> @@ -0,0 +1,24 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2022, Google Inc.<br>
> + *<br>
> + * pipeline.tp - Tracepoints for pipelines<br>
> + */<br>
> +<br>
> +#include "libcamera/internal/tracepoints.h"<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_ipa_call_begin(const char *pipe, const char *func)<br>
> +{<br>
> +     // TODO: Consider TRACE_EVENT_BEGIN<br>
<br>
>From what I've read, I think TRACE_EVENT_BEGIN is better.<br>
<br>
In the perfetto tracepoints generator we could automatically detect<br>
_begin and _end suffixes and convert those to TRACE_EVENT_BEGIN and<br>
TRACE_EVENT_END.<br>
<br></blockquote><div><br></div><div>We might need to add an argument to set the track id, or else Perfetto</div><div>library doesn't know which `begin` the `end` belongs to.</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">
> +     TRACE_EVENT("libcamera", "ipa_call_begin",<br>
> +                 "pipeline_name", pipe,<br>
> +                 "function_name", func);<br>
> +}<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_ipa_call_end(const char *pipe, const char *func)<br>
> +{<br>
> +     // TODO: Consider TRACE_EVENT_END<br>
> +     TRACE_EVENT("libcamera", "ipa_call_end",<br>
> +                 "pipeline_name", pipe,<br>
> +                 "function_name", func);<br>
> +}<br>
> diff --git a/src/libcamera/perfetto/request_perfetto.cpp b/src/libcamera/perfetto/request_perfetto.cpp<br>
> new file mode 100644<br>
> index 00000000..2cfff28e<br>
> --- /dev/null<br>
> +++ b/src/libcamera/perfetto/request_perfetto.cpp<br>
> @@ -0,0 +1,73 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2022, Google Inc.<br>
> + *<br>
> + * request.tp - Tracepoints for the request object<br>
> + */<br>
> +<br>
> +#include <libcamera/framebuffer.h><br>
> +<br>
> +#include "libcamera/internal/request.h"<br>
> +#include "libcamera/internal/tracepoints.h"<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request(libcamera::Request *req)<br>
> +{<br>
> +     TRACE_EVENT("libcamera", "request",<br>
> +                 "request_ptr", reinterpret_cast<uintptr_t>(req),<br>
> +                 "cookie", req->cookie(),<br>
> +                 "status", req->status()); // TODO<br>
<br>
(What is this TODO for?)<br>
<br>
> +}<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_construct(libcamera::Request *req)<br>
> +{<br>
> +     TRACE_EVENT("libcamera", "request_construct",<br>
> +                 "request_ptr", reinterpret_cast<uintptr_t>(req));<br>
> +}<br>
<br>
So in lttng we have a TRACEPOINT_EVENT_CLASS, which defines arguments<br>
and fields and how to obtain the fields from the args. We can<br>
instantiate tracepoints with TRACEPOINT_EVENT_INSTANCE, where we only<br>
have to specify the args, and the fields are already defined and come<br>
from the class. This reduces duplication in the tracepoint definitions.<br>
<br>
I'm wondering from what you've implemented here that perfetto doesn't<br>
support this? I don't see anything in the perfetto docs that suggest<br>
that this is possible either. Although we can mitigate this issue by<br>
generating the perfetto tracepoints :) (this is another reason why I<br>
think we should use the lttng tracepoints definition as the main one and<br>
generate the perfetto ones from those)<br>
<br></blockquote><div><br></div><div>I've discussed with ChromeOS Perfetto TL chinglinyu@, and we found that</div><div>there are limited features in lttng [1], and if we use the lttng tracepoints</div><div>definition as the source of truth, it's hard/weird to implement some Perfetto</div><div>concepts, like nested slices (lifetime of trace events) and flows [2].</div><div><br></div><div>It's not impossible, as you mentioned above, that we could detect the prefix</div><div>and suffix of each libcamera/lttng macro. However, it's a weird code parser</div><div>and generator design.</div><div><br></div><div>[1]: <a href="https://lttng.org/man/3/lttng-ust/v2.8/" target="_blank">https://lttng.org/man/3/lttng-ust/v2.8/</a></div><div>[2]: <a href="https://perfetto.dev/docs/instrumentation/track-events" target="_blank">https://perfetto.dev/docs/instrumentation/track-events</a></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>
> +void LIBCAMERA_TRACE_EVENT_request_destroy(libcamera::Request *req)<br>
> +{<br>
> +     TRACE_EVENT("libcamera", "request_destroy",<br>
> +                 "request_ptr", reinterpret_cast<uintptr_t>(req));<br>
> +}<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_reuse(libcamera::Request *req)<br>
> +{<br>
> +     TRACE_EVENT("libcamera", "request_reuse",<br>
> +                 "request_ptr", reinterpret_cast<uintptr_t>(req));<br>
> +}<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_queue(libcamera::Request *req)<br>
> +{<br>
> +     TRACE_EVENT("libcamera", "request_queue",<br>
> +                 "request_ptr", reinterpret_cast<uintptr_t>(req));<br>
> +}<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_device_queue(libcamera::Request *req)<br>
> +{<br>
> +     TRACE_EVENT("libcamera", "request_device_queue",<br>
> +                 "request_ptr", reinterpret_cast<uintptr_t>(req));<br>
> +}<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_complete(libcamera::Request::Private *req)<br>
> +{<br>
> +     TRACE_EVENT("libcamera", "request_complete",<br>
> +                 "request_private_ptr", reinterpret_cast<uintptr_t>(req));<br>
> +}<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_cancel(libcamera::Request::Private *req)<br>
> +{<br>
> +     TRACE_EVENT("libcamera", "request_cancel",<br>
> +                 "request_private_ptr", reinterpret_cast<uintptr_t>(req));<br>
> +}<br>
> +<br>
> +void LIBCAMERA_TRACE_EVENT_request_complete_buffer(<br>
> +     libcamera::Request::Private *req,<br>
> +     libcamera::FrameBuffer *buf)<br>
> +{<br>
> +     TRACE_EVENT("libcamera", "request_complete_buffer",<br>
> +                 "request_private_ptr", reinterpret_cast<uintptr_t>(req),<br>
> +                 "cookie", req->_o<libcamera::Request>()->cookie(),<br>
> +                 "status", req->_o<libcamera::Request>()->status(), // TODO<br>
> +                 "buffer_ptr", reinterpret_cast<uintptr_t>(buf),<br>
> +                 "buffer_status", buf->metadata().status); // TODO<br>
> +}<br>
<br>
Alright, I'm going to use this as an example.<br>
<br>
First, the tracepoint definition. In lttng we have:<br>
<br>
TRACEPOINT_EVENT(<br>
        libcamera,<br>
        request_complete_buffer,<br>
        TP_ARGS(<br>
                libcamera::Request::Private *, req,<br>
                libcamera::FrameBuffer *, buf<br>
        ),<br>
        TP_FIELDS(<br>
                ctf_integer_hex(uintptr_t, request, reinterpret_cast<uintptr_t>(req))<br>
                ctf_integer(uint64_t, cookie, req->_o<libcamera::Request>()->cookie())<br>
                ctf_integer(int, status, req->_o<libcamera::Request>()->status())<br>
                ctf_integer_hex(uintptr_t, buffer, reinterpret_cast<uintptr_t>(buf))<br>
                ctf_enum(libcamera, buffer_status, uint32_t, buf_status, buf->metadata().status)<br>
        )<br>
)<br>
<br>
And to raise (idk the right verb) the tracepoint, we use<br>
tracepoint(libcamera, request_complete_buffer, request_ptr, buffer_ptr).<br>
In perfetto as far as I understand, it's raised using TRACE_EVENT(...)<br>
and you have to specify everything that lttng wraps away in TP_FIELDS in<br>
the tracepoint definition itself. For this reason, I think it's good to<br>
have the intermediary functions.<br>
<br>
The idea is to generate both the perfetto tracepoint definition and the<br>
intermediary function from the lttng definition. I've studied the<br>
perfetto tracepoint definition docs a bit more than last time, and I<br>
think that going from lttng's definitions to perfetto's is better.<br>
<br>
I was going to walk through an example, but I'm not sure how because it<br>
seems pretty straightforward to me. We can parse the TRACEPOINT_EVENT,<br>
and extract the category name and tracepoint name into the function name<br>
and TRACE_EVENT parameters. TP_ARGS also can get parsed and copied into<br>
the function parameters almost as-is. TP_FIELDS needs a bit of<br>
transformation, but it's just extracting the field name and<br>
transformation, and then adding an extra cast for the type. ctf_enum<br>
would be a bit more involved though, but we have definitions in .tp<br>
files for the enums that map the integer values to strings, so we can<br>
use that. It might involve generating an intermediate file for all the<br>
enums, or I guess we could just generate enum definitions and embed that<br>
in the perfetto tracepoint function definitions cpp file.<br>
<br>
For TRACEPOINT_EVENT_CLASS and TRACEPOINT_EVENT_INSTANCE there's just<br>
one extra level of indirection to obtain TP_FIELDS, but otherwise we<br>
just match it based on the tracepoint class name.<br>
<br>
As for "how", we can use either ply or a custom-made simple C parser for<br>
parsing, and then we'd have to make a custom code generator. If we go<br>
homemade C parser route it doesn't have to be too complex, as we only<br>
have TRACEPOINT_EVENT, TRACEPOINT_EVENT_CLASS, and<br>
TRACEPOINT_EVENT_INSTANCE to parse, and the complex expressions are<br>
simply copied. And then for the code generation part we can use jinja2.<br>
imo the most difficult part is plumbing it through meson, but we've done<br>
that before in the IPC layer, so that has examples of how to do code<br>
generation -> compilation via meson [2] [3] [4], as well as examples for<br>
jinja2 (although it's actually embedded in mojom).<br>
<br></blockquote><div><br></div><div>I agree that in the current functionalities, it's not that hard to implement</div><div>a homemade C/python parser, and generate Perfetto code with jinja2.</div><div>However, we not only "limit" the functionalities of Perfetto with lttng's</div><div>definition, but also add another layer to be maintained.</div><div>It's true that having duplication of libcamera tracepoint definitions </div><div>requires more maintenance work when updating the libcamera</div><div>tracepoints, while the effort to maintain the parser and code generation</div><div>is also non-negligible.</div><div><br></div><div>I also think that creating a new language from which we generate</div><div>tracepoints for both is an overkill though. It seems that we don't have</div><div>a perfect way to support both lttng and Perfetto...</div><div><br></div><div>BTW, I tried lex and yacc to parse lttng macros, and tbh it's very painful</div><div>that it's very hard to use lex to recognize C/C++ type as a token... So</div><div>yeah if we end up deciding to take this route, a homemade parser from</div><div>scratch makes more sense to me.</div><div><br></div><div>So let me list the approaches with pros and cons:</div><div>1. Generate Perfetto code from lttng macros.<br>  Pros: Easy to update libcamera tracepoint macros.</div><div>  Cons: Add maintenance work with the additional parser and code </div><div>generator; hard to implement Perfetto features that lttng doesn't have.</div><div><br></div><div>2. Creating a new language from which we generate both Perfetto</div><div>and lttng macros.</div><div>  Pros: Easy to update libcamera tracepoint macros.</div><div>  Cons: Probably an overkill; hard to cover all features from both </div><div>Perfetto and lttng.</div><div><br></div><div>3. As this patch implements: have duplicated libcamera tracepoint </div><div>definitions.</div><div>  Pros: Easy to add libcamera tracepoint macros to cover all features</div><div>from Perfetto & lttng.</div><div>  Cons: Developer needs to update both sides when the definition of</div><div>libcamera tracepoints changes; Unused macro definitions might be</div><div>adopted in both sides, if a feature is only available in one trace library.</div><div><br></div><div>4. Add another set of libcamera tracepoints for Perfetto, or just use </div><div>Perfetto macros directly in libcamera's source code.</div><div>  Pros: Two isolated trace stacks that don't influence each other. All</div><div>features can be implemented easily.</div><div>  Cons: Duplicated trace code in source code that might lead to </div><div>confusion; developers also need to update both macros when a</div><div>new trace is needed.</div><div><br></div><div>I think approach 3rd and 4th are easier to maintain in the long</div><div>term. WDYT?</div><div><div><br>Also, I'd like to know if you think Perfetto will be used in other linux</div><div>distributions than ChromeOS or Android.</div><div><br></div></div><div>Chinglin, please help amend anything I missed as well :)</div><div><br></div><div>BR,</div><div>Harvey</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">
[2] utils/ipc/meson.build<br>
[3] include/libcamera/ipa/meson.build<br>
[4] src/libcamera/ipa/meson.build<br>
<br>
Would you be able to do this?<br>
<br>
<br>
Paul<br>
<br>
> diff --git a/src/libcamera/tracepoints.cpp b/src/libcamera/tracepoints.cpp<br>
> index 0173b75a..a07ea531 100644<br>
> --- a/src/libcamera/tracepoints.cpp<br>
> +++ b/src/libcamera/tracepoints.cpp<br>
> @@ -4,7 +4,18 @@<br>
>   *<br>
>   * tracepoints.cpp - Tracepoints with lttng<br>
>   */<br>
> +<br>
> +#if HAVE_PERFETTO<br>
> +<br>
> +#include "libcamera/internal/tracepoints.h"<br>
> +<br>
> +PERFETTO_TRACK_EVENT_STATIC_STORAGE();<br>
> +<br>
> +#else<br>
> +<br>
>  #define TRACEPOINT_CREATE_PROBES<br>
>  #define TRACEPOINT_DEFINE<br>
>  <br>
>  #include "libcamera/internal/tracepoints.h"<br>
> +<br>
> +#endif /* HAVE_PERFETTO */<br>
> -- <br>
> 2.39.0.314.g84b9a713c41-goog<br>
> <br>
</blockquote></div></div>
</blockquote></div>