Yeah this is fine. But also maybe let’s just unconditionally skip the demos? I’m really sceptical anyone cares about them and it bloats FFmpeg’s closure and it really does not seem worth it to do split outputs here.

I don’t think we need to patch SDL_CFLAGS etc.; we can just override them in makeFlags. Just patching install should suffice.

not offering the demos is valid. however, we need to patch away the SDL_CFLAGS because otherwise pkg-config crashes and the build fails.

Can’t we just use makeFlags rather than patching the source?

Can’t we just use makeFlags rather than patching the source?

good call, i keep forgetting how cursed make is in just letting you override stuff by command line flags. This does indeed seem to work.

I originally packaged it for the demos, the library is just the supporting machinery for those. Although, it is true that qrtest is more useful than the camera integrations, and qrtest is installed in a way unaffected by this PR.

Apparently ffmpeg has videofilter quirc, which tries to add QR-code content from the video to video metadata? No idea how to use that.

@7c6f434c does this mean you actually use and maintain those demos?
I am happy to split into quirc and quirc-minimal, that was what i proposed initially until emily told me to just drop it. But if there is active users and maintainers, a split might actually be reasonable again. I can easily cherry-pick that, if desired.

Nowadays I am no longer on the hook for handling large amount of 2D-coded paper documents, so fine with me to drop GUI demos.

If you then try to also drop qrtest (no additional dependencies there), though, then remove me from maintainers.

Oh i didn't ever plan on dropping qrtest - the whole point of this is drop the SDL1 dependency going into ffmpeg, because ffmpeg is important and SDL1 is crusty and old. There is absolutely no reason to drop something that doesn't pull deps.

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 389124

x86_64-linux

I am confused if there is work left to do here or not. LGTM and I would merge it.

I am confused if there is work left to do here or not. LGTM and I would merge it.

this depends on whether we go emilys route (drop the demos outright), in which case this is ready. Or we go my original route adding a -minimal target to use in ffmpeg. I am happy with either, just personally not sure which one would be better.

the whole point of this is drop the SDL1 dependency going into ffmpeg

Am I missing something? This should not be in ffmpeg, only ffmpeg-full. We don't really care about a little closure bloat in ffmpeg-full; that's the whole point.

Dropping old crufty demos that rely on SDL1 seems like a good idea regardless though.

Yeah, i only realized after opening the PR this didn't go into ffmpeg, only ffmpeg-full. So this is indeed less impact than i thought initially.

This broke quirc on darwin:

Running phase: installPhase
install flags: SHELL=/nix/store/hmffg6n6ylbl4c30pqc9i71mwqzrd0iv-bash-5.2p37/bin/bash PREFIX=\$\(out\) SDL_CFLAGS= SDL_LIBS= -o inspect -o quirc-demo install
clang -Ilib -O3 -Wall -fPIC  -o demo/camera.o -c demo/camera.c
demo/camera.c:29:10: fatal error: 'linux/videodev2.h' file not found
   29 | #include <linux/videodev2.h>
      |          ^~~~~~~~~~~~~~~~~~~
1 error generated.
make: *** [Makefile:90: demo/camera.o] Error 1

This broke quirc on darwin:

Can you run #389753 through a builder? This is a partial revert (which does keep the SDL drop), and should fix it. But i can't really test.