On Sat, May 31, 2025 at 12:56 AM softworkz .
<softworkz-at-hotmail....@ffmpeg.org> wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of Ramiro
> > Polla
> > Sent: Freitag, 30. Mai 2025 12:52
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: [FFmpeg-devel] [PATCH v3 1/2] fftools/resources: clean up and
> > improve
> > resource manager build system
> >
> > - move .gitignore entries to main .gitignore;
> > - move vpath directives to main Makefile;
> > - remove superfluous comments;
> > - turn css minification sed command into a one-liner;
> > - deduplicate targets depending on CONFIG_RESOURCE_COMPRESSION;
> > - introduce common .res pattern for resource files;
> > - remove RESOURCEOBJS noop from common.mak (it was never populated);
> > - add fftools/graph/Makefile;
> > - rename OBJS-resman to RESMAN-OBJS for consistency;
> > - move graph.{css,html} to fftools/graph/graphprint.{css,html};
> > - disable dependency checking for resource files, to prevent spurious
> > rebuilds;
> > - generate resources list at build-time based on all resource files;
> > - the resource manager now uses the resource filename instead of an ID.
> >
> > Adding resource files now works from any subdir. Suppose you want to
> > add a resource file named "foo.html", then all you have to do is:
> >
> > OBJS-$(CONDITION) += foo.html.res.o
> >
> > To access the resource, you retrieve it by its name:
> >
> > data = ff_resman_get_string("foo.html");
> > ---
>
> Hi Ramiro,
>
> here's my review:
>
> 1. General
>
> First of all, I think there are a bit too many different changes at once
> in this patch. It would be better to have each kind of change
> separate and then apply it uniformly to both, .ptx and .res compression
It would be better if this cleanup work wasn't needed in the first
place. I don't want to pollute git history with half a dozen tiny
fixes to one commit.
I already said I'll deal with ptx later.
> (where applicable). Having different logic for both seems pretty odd,
> that's why I had talked to Timo about his opinion and we had agreed to
> still keep the ptx files, but treat the others as intermediate.
> That applied to the .SECONDARY and CCDEPS setting, and in your patch
> it would also apply to the replacement of the ifdef blocks with the
> inline condition. I'm not against this change, even though will make
> it quite hard for others to understand, when dealing with it in
> the future.
If others find it quite hard to understand conditional substitution,
then perhaps they shouldn't be dealing with Makefiles.
[...]
> 4. fftools/resources/Makefile: #.SECONDARY line
>
> It does not retain min file when uncommented and resource compression is
> enabled.
It does.
> 5. Is this a suitable pattern for a generalized resource handling?
>
> You recently asked me about a plan for this, and I said I don't have
> one yet and that's why I wanted to keep things flexible for now.
> It's not that I haven't thought about it - it's not as easy as one
> might think.
> The solution you are proposing doesn't seem suitable to fulfill these
> requirements - for the following reasons:
[...]
Ok, let's try something simpler... New patch attached.
Ramiro
From a809ff8b920c3d514368c15ba6b6b415d9deb601 Mon Sep 17 00:00:00 2001
From: Ramiro Polla <ramiro.po...@gmail.com>
Date: Tue, 27 May 2025 00:13:47 +0200
Subject: [PATCH v4] fftools/resources: clean up resource manager build system
- move vpath directives to main Makefile;
- remove superfluous comments;
- turn css minification sed command into a one-liner;
- deduplicate targets depending on CONFIG_RESOURCE_COMPRESSION;
- introduce common .res pattern for resource files;
- remove RESOURCEOBJS noop from common.mak (it was never populated);
- rename OBJS-resman to RESMAN-OBJS for consistency;
- disable dependency checking for resource files, to prevent spurious
rebuilds.
---
Makefile | 2 ++
ffbuild/common.mak | 44 ++++++++----------------------------
fftools/Makefile | 2 +-
fftools/resources/.gitignore | 9 +++-----
fftools/resources/Makefile | 14 +++++-------
fftools/resources/resman.c | 12 +++++-----
6 files changed, 27 insertions(+), 56 deletions(-)
diff --git a/Makefile b/Makefile
index 877b0071f6..ed456cf071 100644
--- a/Makefile
+++ b/Makefile
@@ -14,6 +14,8 @@ vpath %.texi $(SRC_PATH)
vpath %.cu $(SRC_PATH)
vpath %.ptx $(SRC_PATH)
vpath %.metal $(SRC_PATH)
+vpath %.html $(SRC_PATH)
+vpath %.css $(SRC_PATH)
vpath %/fate_config.sh.template $(SRC_PATH)
TESTTOOLS = audiogen videogen rotozoom tiny_psnr tiny_ssim base64 audiomatch
diff --git a/ffbuild/common.mak b/ffbuild/common.mak
index ddf48923ea..dbd99fffa2 100644
--- a/ffbuild/common.mak
+++ b/ffbuild/common.mak
@@ -139,45 +139,20 @@ else
$(BIN2C) $(patsubst $(SRC_PATH)/%,$(SRC_LINK)/%,$<) $@ $(subst .,_,$(basename $(notdir $@)))
endif
-# 1) Preprocess CSS to a minified version
%.css.min: TAG = SED
%.css.min: %.css
- $(M)sed 's!/\\*.*\\*/!!g' $< \
- | tr '\n' ' ' \
- | tr -s ' ' \
- | sed 's/^ //; s/ $$//' \
- > $@
+ $(M)sed 's!/\\*.*\\*/!!g' $< | tr '\n' ' ' | tr -s ' ' | sed 's/^ //; s/ $$//' > $@
-ifdef CONFIG_RESOURCE_COMPRESSION
-
-# 2) Gzip the minified CSS
-%.css.min.gz: TAG = GZIP
-%.css.min.gz: %.css.min
- $(M)gzip -nc9 $< > $@
-
-# 3) Convert the gzipped CSS to a .c array
-%.css.c: %.css.min.gz $(BIN2CEXE)
- $(BIN2C) $< $@ $(subst .,_,$(basename $(notdir $@)))
-
-# 4) Gzip the HTML file (no minification needed)
-%.html.gz: TAG = GZIP
-%.html.gz: %.html
+%.res.gz: TAG = GZIP
+%.res.gz: %
$(M)gzip -nc9 $< > $@
-# 5) Convert the gzipped HTML to a .c array
-%.html.c: %.html.gz $(BIN2CEXE)
+%.res.c: %$(CONFIG_RESOURCE_COMPRESSION:yes=.res.gz) $(BIN2CEXE)
$(BIN2C) $< $@ $(subst .,_,$(basename $(notdir $@)))
-else # NO COMPRESSION
-
-# 2) Convert the minified CSS to a .c array
-%.css.c: %.css.min $(BIN2CEXE)
- $(BIN2C) $< $@ $(subst .,_,$(basename $(notdir $@)))
-
-# 3) Convert the plain HTML to a .c array
-%.html.c: %.html $(BIN2CEXE)
- $(BIN2C) $< $@ $(subst .,_,$(basename $(notdir $@)))
-endif
+# Disable dependency checking to prevent spurious rebuilds
+%.res.o: CCDEP =
+%.res.o: CC_DEPFLAGS =
clean::
$(RM) $(BIN2CEXE) $(CLEANSUFFIXES:%=ffbuild/%)
@@ -229,10 +204,9 @@ SKIPHEADERS += $(ARCH_HEADERS:%=$(ARCH)/%) $(SKIPHEADERS-)
SKIPHEADERS := $(SKIPHEADERS:%=$(SUBDIR)%)
HOBJS = $(filter-out $(SKIPHEADERS:.h=.h.o),$(ALLHEADERS:.h=.h.o))
PTXOBJS = $(filter %.ptx.o,$(OBJS))
-RESOURCEOBJS = $(filter %.css.o %.html.o,$(OBJS))
$(HOBJS): CCFLAGS += $(CFLAGS_HEADERS)
checkheaders: $(HOBJS)
-.SECONDARY: $(HOBJS:.o=.c) $(PTXOBJS:.o=.c) $(PTXOBJS:.o=.gz) $(PTXOBJS:.o=) $(RESOURCEOBJS:.o=.c) $(RESOURCEOBJS:%.css.o=%.css.min) $(RESOURCEOBJS:%.css.o=%.css.min.gz) $(RESOURCEOBJS:%.html.o=%.html.gz) $(RESOURCEOBJS:.o=)
+.SECONDARY: $(HOBJS:.o=.c) $(PTXOBJS:.o=.c) $(PTXOBJS:.o=.gz) $(PTXOBJS:.o=)
alltools: $(TOOLS)
@@ -252,7 +226,7 @@ $(TOOLOBJS): | tools
OUTDIRS := $(OUTDIRS) $(dir $(OBJS) $(HOBJS) $(HOSTOBJS) $(SHLIBOBJS) $(STLIBOBJS) $(TESTOBJS))
-CLEANSUFFIXES = *.d *.gcda *.gcno *.h.c *.ho *.map *.o *.objs *.pc *.ptx *.ptx.gz *.ptx.c *.ver *.version *.html.gz *.html.c *.css.gz *.css.c *$(DEFAULT_X86ASMD).asm *~ *.ilk *.pdb
+CLEANSUFFIXES = *.d *.gcda *.gcno *.h.c *.ho *.map *.o *.objs *.pc *.ptx *.ptx.gz *.ptx.c *.ver *.version *.css.min *.res.gz *.res.c *$(DEFAULT_X86ASMD).asm *~ *.ilk *.pdb
LIBSUFFIXES = *.a *.lib *.so *.so.* *.dylib *.dll *.def *.dll.a
define RULES
diff --git a/fftools/Makefile b/fftools/Makefile
index c1eba733da..42821a698b 100644
--- a/fftools/Makefile
+++ b/fftools/Makefile
@@ -35,7 +35,7 @@ OBJS-ffmpeg += \
fftools/textformat/tw_avio.o \
fftools/textformat/tw_buffer.o \
fftools/textformat/tw_stdout.o \
- $(OBJS-resman) \
+ $(RESMAN-OBJS) \
OBJS-ffprobe += \
fftools/textformat/avtextformat.o \
diff --git a/fftools/resources/.gitignore b/fftools/resources/.gitignore
index bda2c59a1c..19facbc590 100644
--- a/fftools/resources/.gitignore
+++ b/fftools/resources/.gitignore
@@ -1,6 +1,3 @@
-*.html.c
-*.css.c
-*.html.gz
-*.css.gz
-*.min
-*.min.gz
+*.css.min
+*.res.c
+*.res.gz
diff --git a/fftools/resources/Makefile b/fftools/resources/Makefile
index 8579a52678..4b6d63c8d2 100644
--- a/fftools/resources/Makefile
+++ b/fftools/resources/Makefile
@@ -1,13 +1,11 @@
clean::
$(RM) $(CLEANSUFFIXES:%=fftools/resources/%)
-vpath %.html $(SRC_PATH)
-vpath %.css $(SRC_PATH)
+RESMAN-OBJS += \
+ fftools/resources/resman.o \
+ fftools/resources/graph.html.res.o \
+ fftools/resources/graph.css.min.res.o \
# Uncomment to prevent deletion during build
-#.PRECIOUS: %.css.c %.css.min %.css.gz %.css.min.gz %.html.gz %.html.c
-
-OBJS-resman += \
- fftools/resources/resman.o \
- fftools/resources/graph.html.o \
- fftools/resources/graph.css.o \
+#RESOBJS = $(filter %.res.o,$(RESMAN-OBJS))
+.SECONDARY: $(RESOBJS:.o=.gz) $(RESOBJS:.o=.c) $(RESOBJS:.res.o=)
diff --git a/fftools/resources/resman.c b/fftools/resources/resman.c
index bce3589169..cdacecd038 100644
--- a/fftools/resources/resman.c
+++ b/fftools/resources/resman.c
@@ -37,15 +37,15 @@
#include "libavutil/dict.h"
#include "libavutil/common.h"
-extern const unsigned char ff_graph_html_data[];
-extern const unsigned int ff_graph_html_len;
+extern const unsigned char ff_graph_html_res_data[];
+extern const unsigned int ff_graph_html_res_len;
-extern const unsigned char ff_graph_css_data[];
-extern const unsigned ff_graph_css_len;
+extern const unsigned char ff_graph_css_min_res_data[];
+extern const unsigned ff_graph_css_min_res_len;
static const FFResourceDefinition resource_definitions[] = {
- [FF_RESOURCE_GRAPH_CSS] = { FF_RESOURCE_GRAPH_CSS, "graph.css", &ff_graph_css_data[0], &ff_graph_css_len },
- [FF_RESOURCE_GRAPH_HTML] = { FF_RESOURCE_GRAPH_HTML, "graph.html", &ff_graph_html_data[0], &ff_graph_html_len },
+ [FF_RESOURCE_GRAPH_CSS] = { FF_RESOURCE_GRAPH_CSS, "graph.css", &ff_graph_css_min_res_data[0], &ff_graph_css_min_res_len },
+ [FF_RESOURCE_GRAPH_HTML] = { FF_RESOURCE_GRAPH_HTML, "graph.html", &ff_graph_html_res_data[0], &ff_graph_html_res_len },
};
--
2.39.5
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
