[FFmpeg-devel] [PATCH 3/4] fftools/graph/graphprint: load CSS and HTML resources from ffmpeg data directories

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Marton
> Balint
> Sent: Samstag, 17. Mai 2025 01:09
> To: ffmpeg-devel at ffmpeg.org
> Cc: Marton Balint <cus at passwd.hu>
> Subject: [FFmpeg-devel] [PATCH 3/4] fftools/graph/graphprint: load CSS and
> HTML resources from ffmpeg data directories
> 
> Similar to how it is done for ffpreset files. This allows the users to change
> the HTML/CSS templates at will.
> 
> Signed-off-by: Marton Balint <cus at passwd.hu>
> ---
>  Makefile                        |  2 +-
>  doc/ffmpeg.texi                 |  4 ++-
>  fftools/cmdutils.c              | 48 +++++++++++++++++++++++++++++++++
>  fftools/cmdutils.h              |  2 ++
>  fftools/graph/graphprint.c      | 16 ++++++++---
>  fftools/textformat/tf_mermaid.h |  4 +--
>  6 files changed, 68 insertions(+), 8 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index e2250f6bc6..ebc50bb510 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -32,7 +32,7 @@ FFLIBS-$(CONFIG_SWSCALE)    += swscale
> 
>  FFLIBS := avutil
> 
> -DATA_FILES := $(wildcard $(SRC_PATH)/presets/*.ffpreset)
> $(SRC_PATH)/doc/ffprobe.xsd
> +DATA_FILES := $(wildcard $(SRC_PATH)/presets/*.ffpreset)
> $(SRC_PATH)/doc/ffprobe.xsd $(SRC_PATH)/fftools/resources/graph.css
> $(SRC_PATH)/fftools/resources/graph.html
> 
>  SKIPHEADERS = compat/w32pthreads.h
> 
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index 35675b5309..8edf188ab9 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -1402,7 +1402,9 @@ Writes execution graph details to the specified file in
> the format set via -prin
> 
>  @item -print_graphs_format @var{format} (@emph{global})
>  Sets the output format (available formats are: default, compact, csv, flat,
> ini, json, xml, mermaid, mermaidhtml)
> -The default format is json.
> +The default format is json. The mermarid and mermaidhtml formats need
> graph.css
> +and graph.html resources which are searched in the FFMPEG data directories,
> +similar to ffpreset files.
> 
>  @item -progress @var{url} (@emph{global})
>  Send program-friendly progress information to @var{url}.
> diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> index 1de670c2e4..01e57d91e8 100644
> --- a/fftools/cmdutils.c
> +++ b/fftools/cmdutils.c
> @@ -1005,6 +1005,54 @@ FILE *get_preset_file(AVBPrint *filename,
>      return f;
>  }
> 
> +int file_read_from_datadir(const char *filename, char **outbuf)
> +{
> +    FILE *f;
> +    char *buf = NULL;
> +    long size;
> +    AVBPrint bp;
> +
> +    av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
> +    f = get_datadir_file_fmt(&bp, "%s", filename);
> +    if (!f)
> +        return AVERROR(errno);
> +
> +    if (fseek(f, 0, SEEK_END)) {
> +        int ret = AVERROR(errno);
> +        fclose(f);
> +        return ret;
> +    }
> +
> +    size = ftell(f);
> +    if (size < 0 || size >= INT_MAX) {
> +        fclose(f);
> +        return AVERROR(EINVAL);
> +    }
> +
> +    if (fseek(f, 0, SEEK_SET)) {
> +        int ret = AVERROR(errno);
> +        fclose(f);
> +        return ret;
> +    }
> +
> +    buf = av_malloc(size + 1);
> +    if (!buf) {
> +        fclose(f);
> +        return AVERROR(ENOMEM);
> +    }
> +
> +    if (fread(buf, 1, size, f) != size) {
> +        fclose(f);
> +        return AVERROR(EIO);
> +    }
> +    buf[size] = 0;
> +
> +    fclose(f);
> +
> +    *outbuf = buf;
> +    return 0;
> +}
> +
>  int cmdutils_isalnum(char c)
>  {
>      return (c >= '0' && c <= '9') ||
> diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
> index 216a2bcfe7..b324e5fc65 100644
> --- a/fftools/cmdutils.h
> +++ b/fftools/cmdutils.h
> @@ -504,6 +504,8 @@ int read_yesno(void);
>  FILE *get_preset_file(AVBPrint *filename,
>                        const char *preset_name, int is_path, const char
> *codec_name);
> 
> +int file_read_from_datadir(const char *filename, char **buf);
> +
>  /**
>   * Realloc array to hold new_size elements of elem_size.
>   *
> diff --git a/fftools/graph/graphprint.c b/fftools/graph/graphprint.c
> index fc94a75797..fbcb7f1b75 100644
> --- a/fftools/graph/graphprint.c
> +++ b/fftools/graph/graphprint.c
> @@ -41,7 +41,6 @@
>  #include "libavutil/hwcontext.h"
>  #include "fftools/textformat/avtextformat.h"
>  #include "fftools/textformat/tf_mermaid.h"
> -#include "fftools/resources/resman.h"
> 
>  typedef enum {
>      SECTION_ID_ROOT,
> @@ -935,10 +934,19 @@ static int init_graphprint(GraphPrintContext **pgpc,
> AVBPrint *target_buf)
>      }
> 
>      if (!strcmp(text_formatter->name, "mermaid") || !strcmp(text_formatter-
> >name, "mermaidhtml")) {
> -        gpc->diagram_config.diagram_css =
> ff_resman_get_string(FF_RESOURCE_GRAPH_CSS);
> +        ret = file_read_from_datadir("graph.css", &gpc-
> >diagram_config.diagram_css);
> +        if (ret < 0) {
> +            av_log(NULL, AV_LOG_ERROR, "graph.css needed for mermaid graph
> format cannot be opened: %s\n", av_err2str(ret));
> +            goto fail;
> +        }
> 
> -        if (!strcmp(text_formatter->name, "mermaidhtml"))
> -            gpc->diagram_config.html_template =
> ff_resman_get_string(FF_RESOURCE_GRAPH_HTML);
> +        if (!strcmp(text_formatter->name, "mermaidhtml")) {
> +            ret = file_read_from_datadir("graph.html", &gpc-
> >diagram_config.html_template);
> +            if (ret < 0) {
> +                av_log(NULL, AV_LOG_ERROR, "graph.html needed for
> mermaridhtml graph format cannot be opened: %s\n", av_err2str(ret));
> +                goto fail;
> +            }
> +        }
> 
>          av_diagram_init(tfc, &gpc->diagram_config);
>      }
> diff --git a/fftools/textformat/tf_mermaid.h b/fftools/textformat/tf_mermaid.h
> index 6e8f2a9b42..0b34d8ab20 100644
> --- a/fftools/textformat/tf_mermaid.h
> +++ b/fftools/textformat/tf_mermaid.h
> @@ -28,8 +28,8 @@ typedef enum {
> 
>  typedef struct AVDiagramConfig {
>      AVDiagramType diagram_type;
> -    const char *diagram_css;
> -    const char *html_template;
> +    char *diagram_css;
> +    char *html_template;
>  } AVDiagramConfig;
> 
> 
> --

NAK - I totally disagree to this change


1. It makes the feature widely unusable. In all those years I've never seen
   or dealt with an "FFmpeg data folder" and I know that everywhere I would
   want to use this feature, there's no such folder nor files

2. The html and the CSS are in no way like exchangeable templates. They are 
   part of the implementation of the feature - especially the CSS.
   Another - yet unsubmitted - feature (data schema graphs) will have its
   own and different html and css, which is also very specific.

3. Even when considering the files as something different than templates,
   this will still not fly. The code in FFmpeg is tightly coupled and 
   dependent of the html and css. When the feature gets updated or extended
   this will almost certainly include changes to the css, which means that
   it would be incompatible with the css from a different FFmpeg version.
   There's no logical independence and hence the files cannot be delivered
   separately


PS: If you look at the html and want to reply that it doesn't appear to be 
    very specific: 
 1. Then look at the CSS
 2. The specific part in the html is the Mermaid version. And there are
    breaking changes across mermaid versions and the CSS depends on the
    Mermaid version as well, which means that these two are also tied 
    together and both in turn with the code.

sw