On Sat, May 31, 2025 at 12:56 AM softworkz .
<softworkz-at-hotmail.com@ffmpeg.org> wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@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