mpv
mpv copied to clipboard
Fix gbm.h/jpeglib.h detection outside of default search path
The following scenario maybe uncommon on Linux but BSDs separate base system and packages to avoid tainting build environment:
- Install libjpeg-turbo under
/myprefix
- Make sure
libjpeg.pc
points to/myprefix/include
- Run
waf configure
- Confirm
JPEG support
detection failed - Check
build/config.log
- Notice
-I/myprefix/include
is missing
When a project provides *.pc file(s) it expects consumers to consult them how to find headers, what flags to use, etc. instead of probing them manually. Only when there's none one can blame BSDs for crippling the default environment. Fortunately, all affected places have *.pc files, so we can drop an ugly wart in FreeBSD CI config.
Not sure what happened to this. Can't we just require pkg-config in all cases?
Can't we just require pkg-config in all cases?
libjpeg.pc
is not provided by IJG libjpeg unlike libjpeg-turbo. shaderc is used by d3d11 on Windows where I'm not sure if pkg-config
is usable.
gbm.pc
maybe missing on non-Mesa implementations but neither nvidia-driver nor raspberrypi-userland support libgbm. No clue about Android vendors.
I doubt that there's non-mesa GBM (if it is, it's going to be some sort of horrible ARM-Linux thing with non-upstreamed drivers, not a big loss). That leaves libjpeg as possibly the only thing that needs a non-pc check? I'm a bit hesitant with needing complicated checks. Also, the new build system is work in progress.
Sorry, after checking IJG libjpeg upstream:
Version 9c 14-Jan-2018
-----------------------
...
Add libjpeg pkg-config file. Thank to Mark Lavi, Vincent Torri,
Patrick McMunn, and Huw Davies for suggestion.
FreeBSD package of IJG jpeg haven't been updated since 2014-12-10 as libjpeg-turbo became default on 2015-09-16
#8208 regressed Wayland support (only with dbc3ab38807c in mind) as <linux/input-event-codes.h>
is now checked with default compiler flags but default only covers base system while the header is part of evdev-proto
package on DragonFly and FreeBSD. <dev/evdev/input-event-codes.h>
is not supposed to be used by portable software.
OK, so I was for a bit confused since the CI was still passing, but it seems like that related change was in this PR indeed :) .
So the history of that PR is as follows:
-
Someone on the mpv-devel IRC channel had netbsd installed, and noticed that it had Wayland stuff available, but compilation would then fail as
linux/input.h
was included. -
Thus the idea came to add a configure check for it, so that the lack of header would cause things already fail at the configure level, and not later in failing compilation.
-
@linkmauve quoted the Wayland documentation as mentioning
linux/input-event-codes.h
as being the upstream of the event identifiers for Wayland instead of the more generic header. This was found to not break the existing CI flow, and work on linux as well (requires Linux 4.4 or so I think where the header was split?) -
Thus a PR was added which: a) changes the header b) adds a configure check (which would have been added in any case since someone noticed compilation was broken and apparently Wayland headers etc were available).
The b) part of step 4 would have been most likely taken anyways, just to make Wayland not get enabled in NetBSD.
I would guesstimate that adding any configure check for the header would fail this, or is the new split header somehow special?
adding any configure check for the header would fail this
Correct. Any header from packages that isn't backed by pkg-config --cflags
.
I'm not sure how to fix this yet.
1. Someone on the mpv-devel IRC channel had netbsd installed, and noticed that it had Wayland stuff available, but compilation would then fail as `linux/input.h` was included.
mpv package in PkgSrc does support Wayland on NetBSD. However, @niacat probably didn't upstream everything which messes up expectations for those who build outside of PkgSrc.
https://github.com/NetBSD/pkgsrc/blob/trunk/multimedia/mpv/patches/patch-video_out_wayland__common.c
Yea, we had something very similar originally in the PR (redefinition of the utilized definitions), but unfortunately there were some conflicting reports on whether an actual Wayland compositor was available, so instead of adding extra ifdefs for "nothing" (since the person on mpv-devel wasn't sure how to test it, either - and FreeBSD seemed to provide the compatibility headers), it was just made to fail configure.
edit: I think the initial end result with regards to what would happen if we would add those re-definitions, those would probably go under a helper header under osdep/
instead. That way the actual wayland code and the re-definitions would be kept separate.
But since the Linux header is the effective upstream, we would totally prefer that to be provided if possible (if some system standardizes on the pkg-config name for Linux compatibility layers, we're 100% open for adding checks for that as far as I can tell - and hopefully for more than one non-Linux *nix system).
Alternatively this should be brought up again to the Wayland folks and it requested for definitions to be duplicated on systems that aren't Linux.
Glancing at this again, the meson build actually does use pkg-config on all the mentioned libraries in here (gbm, jpeg, and shaderc) so this is solved really. The wayland thing is separate of course (and there's another PR for that). I'll go ahead and close this.