SPEC: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/02092337-intel-media-driver-free/intel-media-driver.spec SRPM: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/02092337-intel-media-driver-free/intel-media-driver-free-20.4.5-1.fc35.src.rpm
Taking this review.
SPEC: https://gist.github.com/frantisekz/2b8070d039b4a14498e61cf16c6c3511 I've taken it off copr for now, until we figure out what needs to be stripped from source tarball.
So, all the dependencies necessary to build a working driver from sources are available in referenced review requests and a copr: https://copr.fedorainfracloud.org/coprs/frantisekz/intel-media-driver-free/ . I have a stripped down src.rpm and spec with ton of patches that glue the driver together. I'll have to take another round of look and find out if there are some more files to be stripped (which seems like it'd need to be a manual process for new driver releases). SPEC: https://gist.githubusercontent.com/frantisekz/2b8070d039b4a14498e61cf16c6c3511/raw/f67659d6b2b518f056f8652d085ed38830dfdb04/intel-media-driver-free.spec SRPM: https://mojefedora.cz/intel-driver-review/intel-media-driver-free-21.4.3-1.fc36.src.rpm
Doesn't this need FE-Legal review, then?
Also, strip.py is mentioned in a comment only, but not included in Source: or src.rpm.
FWIW, the driver from your COPR is working for me (Intel 11th gen/Tiger Lake) well enough to play some videos using mpv with VAAPI hwaccel, which is really great. And with Firefox 96, VAAPI accelerated video playback is no longer crashing, either.
Sorry for the accidental NEEDINFO, Neal.
SPEC: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/03191953-intel-media-driver-free/intel-media-driver.spec SRPM: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/03191953-intel-media-driver-free/intel-media-driver-free-22.1.1-1.fc36.src.rpm
Love to see that, Thanks!
BuildRequires: pkgconfig(pciaccess) BuildRequires: libpciaccess Isn't that redundant? libpciaccess-devel requires libpciaccess. Please sort the BuildRequires: alphabetically. # Remove pre-built (but unused) files rm -f Tools/MediaDriverTools/UMDPerfProfiler/MediaPerfParser -> Please use rm -v or at least drop the -f. You won't notice if they drop that file upstream otherwise. %{_libdir}/libigfxcmrt.so* Specify SONAME explicitly, please. If there are upstream bug reports for the patches, please mention them in comments.
Just for understanding: would it still be possible to request this to be part of a default setup? Or would that need to wait for Fedora 36?
(In reply to Robert Mader from comment #11) > Just for understanding: would it still be possible to request this to be > part of a default setup? Or would that need to wait for Fedora 36? Most of its dependencies are F36+ only, so this will be too.
> # Original source has non-free and/or patented files, use strip.py on original source! Where is strip.py?
Sorry, I've been on PTO for some time and then busy with other stuff. Some updates: - upstream situation has improved a lot allowing me to drop all the "Wtf-*" patches: https://github.com/intel/media-driver/pull/1328 - it's now blocked on https://github.com/intel/media-driver/issues/1356 , but the team working on media-driver is responding a fixing stuff pretty quickly - I'll post updated spec/srpm once the issue above gets fixed somehow - strip.py would no longer be needed, there's upstream documentation ( https://github.com/intel/media-driver/wiki/Media-Driver-Shaders-(EU-Kernels)#build-with-open-source-shaders ) on how to remove non-free/patented files, which is basically: #find . -name kernel | grep gen | xargs rm -r #find . -name cm_gpucopy_kernel* | xargs rm #find . -name cmrt_kernel | xargs rm -r
Has there been any progress on this package? I've upgraded one of my machines to F36 and I'm able to test this on hardware supported by intel-media-driver only (Intel Tiger Lake).
Firefox for Fedora comes with enabled VA-API by default now (https://bodhi.fedoraproject.org/updates/FEDORA-2022-a66739c5ad) so would be great to have this package in Fedora.
SPEC: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/04991856-intel-media-driver-free/intel-media-driver.spec SRPM: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/04991856-intel-media-driver-free/intel-media-driver-free-22.6.0-1.fc38.src.rpm strip.py: https://gist.github.com/frantisekz/137f3d213ec6392b51def31255456f78 Patch to remove non-free codecs support entirely: https://gist.github.com/frantisekz/4932cf5b5b304163268c6c7386f61d0b This should(tm) be ready for legal and package review. THe driver is very barebones as we're currently skipping kernel compilation entirely ( https://github.com/intel/media-driver/issues/1356 ).
> # This is an Intel only vaapi backend > ExclusiveArch: i686 x86_64 With Intel Arc GPUs now a thing, is this really x86-only now? You could use it with ARM or POWER based systems too...
Frantisek, it's great to see a progress here!
(In reply to Neal Gompa from comment #18) > > # This is an Intel only vaapi backend > > ExclusiveArch: i686 x86_64 > > With Intel Arc GPUs now a thing, is this really x86-only now? You could use > it with ARM or POWER based systems too... The problem is this: https://src.fedoraproject.org/rpms/intel-gmmlib/blob/rawhide/f/intel-gmmlib.spec#_11 Also, once the time comes to enable free kernels compilation, that'd require bunch of igc, cm, compute-runtime stuff (or leave the kernels off for non-x86 forever). We can get this in in x86 only and figure path forward later on. I remember I'd problems with bunch of Intel stuff regressing pretty often on non-x86 compared to x86 in the past.
SPEC: https://mojefedora.cz/intel-driver-review/intel-media-driver.spec SRPM: https://mojefedora.cz/intel-driver-review/intel-media-driver-free-22.6.0-1.fc38.src.rpm strip.py: https://gist.github.com/frantisekz/137f3d213ec6392b51def31255456f78 Patch to remove non-free codecs support entirely: https://gist.github.com/frantisekz/4932cf5b5b304163268c6c7386f61d0b
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated Issues: ======= - Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files directly in %_libdir. See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#_devel_packages - Spec file name must match the spec package %{name}, in the format %{name}.spec. Note: intel-media-driver.spec should be intel-media-driver-free.spec See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#_spec_file_naming ===== MUST items ===== C/C++: [x]: Provides: bundled(gnulib) in place as required. Note: Sources not installed [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: ldconfig not called in %post and %postun for Fedora 28 and later. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: There is no build directory. Running licensecheck on vanilla upstream sources. No licenses found. Please check the source files for licenses manually. [x]: License file installed when any subpackage combination is installed. [x]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [-]: Package is not known to require an ExcludeArch tag. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 20480 bytes in 1 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [x]: SourceX tarball generation or download is documented. Note: Package contains tarball without URL, check comments [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Fully versioned dependency in subpackages if applicable. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: SourceX is a working URL. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [!]: Spec file according to URL is the same as in SRPM. Note: Bad spec filename: /home/ngompa/1942132-intel-media-driver/srpm- unpacked/intel-media-driver.spec See: (this test has no URL) [x]: Rpmlint is run on debuginfo package(s). Note: There are rpmlint messages (see attachment). [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. Rpmlint ------- Cannot parse rpmlint output: Rpmlint (debuginfo) ------------------- Cannot parse rpmlint output: Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 3 intel-media-driver-free.x86_64: W: incoherent-version-in-changelog 21.4.3-1 ['22.6.0-1.fc38', '22.6.0-1'] intel-media-driver-free.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libigfxcmrt.so 3 packages and 0 specfiles checked; 0 errors, 2 warnings, 0 badness; has taken 2.5 s Unversioned so-files -------------------- intel-media-driver-free: /usr/lib64/dri/iHD_drv_video.so intel-media-driver-free: /usr/lib64/libigfxcmrt.so Requires -------- intel-media-driver-free (rpmlib, GLIBC filtered): libc.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3.1)(64bit) libigdgmm.so.12()(64bit) libigfxcmrt.so.7()(64bit) libm.so.6()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.5)(64bit) libstdc++.so.6(CXXABI_1.3.8)(64bit) libva.so.2()(64bit) libva.so.2(VA_API_0.33.0)(64bit) rtld(GNU_HASH) intel-media-driver-free-debuginfo (rpmlib, GLIBC filtered): intel-media-driver-free-debugsource (rpmlib, GLIBC filtered): Provides -------- intel-media-driver-free: intel-media-driver-free intel-media-driver-free(x86-64) libigfxcmrt.so.7()(64bit) metainfo() metainfo(intel-media-driver.metainfo.xml) intel-media-driver-free-debuginfo: debuginfo(build-id) intel-media-driver-free-debuginfo intel-media-driver-free-debuginfo(x86-64) libigfxcmrt.so.7.2.0-22.6.0-1.fc38.x86_64.debug()(64bit) intel-media-driver-free-debugsource: intel-media-driver-free-debugsource intel-media-driver-free-debugsource(x86-64) Generated by fedora-review 0.9.0 (6761b6c) last change: 2022-08-23 Command line :/usr/bin/fedora-review -b 1942132 -m fedora-rawhide-x86_64 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, C/C++, Shell-api Disabled plugins: Ocaml, Python, SugarActivity, Java, fonts, PHP, Perl, R, Haskell Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
Provides -------- intel-media-driver-free: [...] libigfxcmrt.so.7()(64bit) Why this library is provided within intel-media-driver-free ? Also you seem to have submitted an alpha version of the project for review. Is this intended ? As a reminder intel-media-driver has a quarter release schedule https://github.com/intel/media-driver/releases So the 22.6.x version will requires higher dependencies (like intel-gmmlib) that will have more chances to break others components (opencl stack)...
(In reply to Nicolas Chauvet (kwizart) from comment #23) > Provides > -------- > intel-media-driver-free: > [...] > libigfxcmrt.so.7()(64bit) > > Why this library is provided within intel-media-driver-free ? > It is part of the driver, but I agree, this needs to be subpackaged out. The development headers and pkgconfig files should also be shipped in a devel package.
Thanks for the review, I am currently waiting for https://github.com/intel/media-driver/issues/1540 to be resolved (when testing the driver, I'd found out that Xe* and DG graphics take completely different code-path and I didn't find a way to patch out HEVC Encode out there yet, having that issue resolved should make it trivial to do so). The 22.6.x would be necessary, there was a lot of refactoring and backporting a fix for the mentioned issue would probably be far from trivial. @kwizart , gmmlib 22.3.1 works fine with compute-runtime that's currently packaged in F38/F37.
SPEC: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/05227720-intel-media-driver-free/intel-media-driver-free.spec SRPM: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/05227720-intel-media-driver-free/intel-media-driver-free-22.6.6-1.fc38.src.rpm
Downstream patch to disable patented codecs is no longer needed, everything is processed via cmake parameters and source strip. HEVC Encoding leak in the Xe codepath was fixed ( https://github.com/intel/media-driver/issues/1540 ), packaging comments should all be addressed (I hope I didn't miss anything). So, apart from upstream fixes: - development files were split out into the -devel subpackage - spec file naming issue fixed I've tested exposed codecs on Gen9, Gen11, Gen12 and Xe, all seem okay (VP8, VP9, AV1) are supported, JPEG Decode was disabled due to https://github.com/intel/media-driver/issues/1540#issuecomment-1381738561 .
Copr build: https://copr.fedorainfracloud.org/coprs/build/5227768 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-1942132-intel-media-driver-free/fedora-rawhide-x86_64/05227768-intel-media-driver-free/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
Please fix the conflicting issue with the full featured. (likely to requires patching libva, but looks saner than the mesa nightmare currently occurring).
(In reply to Nicolas Chauvet (kwizart) from comment #29) > Please fix the conflicting issue with the full featured. > (likely to requires patching libva, but looks saner than the mesa nightmare > currently occurring). Well, the -free/-full split is an upstream intel-media-driver thing, not a libva thing. I've filed a ticket to ask for this: https://github.com/intel/media-driver/issues/1598 But I won't hold up this package on that unless I get an assertion that it'd happen reasonably quickly.
(In reply to Nicolas Chauvet (kwizart) from comment #29) > Please fix the conflicting issue with the full featured. > (likely to requires patching libva, but looks saner than the mesa nightmare > currently occurring). I'd rather not block getting this into Fedora on paralel instability between the full and free versions of the package. A user should always be able to dnf swap and use the full version of the driver as a short term solution, at least since we're now talking about just getting this into Fedora, not default presence in Workstation/KDE/... installations, imo. I am definitely planning to open a ticket against this package to figure out some solution for this issue, should this get approved. There are bunch of options on how to achieve a proper solution, apart from those proposed in the libva ticket ( https://github.com/intel/libva/issues/639 ) and the ticket that Neal created, I had something like what we're doing with wine-dxvk in mind. Short description: wine package creates alternatives handle for various so files with lower priority in post-inst, wine-dxvk adds another alternatives to that handle with higher priorities. The only drawback there that I can think of is that user would have to have free version installed in order to be able to install the full version (the free version would own the alternatives handle, the full version would have a higher priority), but Requires: intel-media-driver-free in the full blown package would easily solve that.
> %{_libdir}/libigfxcmrt.so* Why isn't the regular shared library split out into a subpackage rather than being in the devel subpackage? It doesn't belong in the main driver package, sure, but it also doesn't belong in the devel subpackage either.
SPEC: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/05620176-intel-media-driver-free/intel-media-driver-free.spec SRPM: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/05620176-intel-media-driver-free/intel-media-driver-free-23.1.3-1.fc39.src.rpm (In reply to Neal Gompa from comment #32) > > %{_libdir}/libigfxcmrt.so* > > Why isn't the regular shared library split out into a subpackage rather than > being in the devel subpackage? It doesn't belong in the main driver package, > sure, but it also doesn't belong in the devel subpackage either. Should be addressed, package bumped to the latest upstream tag.
Created attachment 1949453 [details] The .spec file difference from Copr build 5227768 to 5620209
Copr build: https://copr.fedorainfracloud.org/coprs/build/5620209 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-1942132-intel-media-driver-free/fedora-rawhide-x86_64/05620209-intel-media-driver-free/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
I don't get why there is a libigfxcmrt sub-package out of this project. Is this used by some other component ? My understanding is that library is a fork of this project: https://koji.fedoraproject.org/koji/buildinfo?buildID=2114125 Now if intel-media-driver is the only user of the library, I don't get the problem to have it bundled at all. It will avoid the need of any ABI computation. Also it will ease the swap between the full featured and Until a solution is found WRT alternative version (https://github.com/intel/libva/issues/639 ), I would be okay to have a downstream patch with the iHD-free driver name added into libva. Please rename as appropriate.
(In reply to Nicolas Chauvet (kwizart) from comment #36) > I don't get why there is a libigfxcmrt sub-package out of this project. Is > this used by some other component ? > > My understanding is that library is a fork of this project: > https://koji.fedoraproject.org/koji/buildinfo?buildID=2114125 It seems like (eg. https://packages.debian.org/unstable/libigfxcmrt-dev ; https://packages.ubuntu.com/search?keywords=libigfxcmrt-dev ). > > Now if intel-media-driver is the only user of the library, I don't get the > problem to have it bundled at all. > It will avoid the need of any ABI computation. That'd be the case, but media-driver isn't the user of the library, this library depends on media-driver (and something else can depend on this lib). I'd say there is no point in having it as a part of intel-media-driver directly, and (the other option) waste to just rm that file (and the entire libigfxcmrt-devel) I guess since it gets compiled anyway? > > Also it will ease the swap between the full featured and I wouldn't say many users would install this library, and if this ever gets to Fedora, we can add something like Requires: intel-media-driver-free or intel-media-driver to it. > > > Until a solution is found WRT alternative version > (https://github.com/intel/libva/issues/639 ), I would be okay to have a > downstream patch with the iHD-free driver name added into libva. > Please rename as appropriate. Okay, I'll keep that in mind, thanks (this is nowhere near legal ack yet afaik)!
(In reply to František Zatloukal from comment #37) > > > > > > Until a solution is found WRT alternative version > > (https://github.com/intel/libva/issues/639 ), I would be okay to have a > > downstream patch with the iHD-free driver name added into libva. > > Please rename as appropriate. > > Okay, I'll keep that in mind, thanks (this is nowhere near legal ack yet > afaik)! I don't believe this is an acceptable route to fix this problem. We should test the environment.d solution before we do that. Upstream advises against patching the binary filename.
Hi all. Just wanted to chime in that I've been testing a system with Intel DG2(Arc AXXX GPUs) and have found that you need 22.6.4 or 23.1.0+ for intel-media-driver in order to get successful hw decoding. Happy to help with testing since not many people have this hardware.
(In reply to Neal Gompa from comment #38) > I don't believe this is an acceptable route to fix this problem. We should > test the environment.d solution before we do that. environment.d doesn't work in MATE, for example (i.e. nothing imports environment from systemd like GNOME or KDE does).
> -DMPEG2_Decode_Supported="no" \ You can turn MPEG2 on.
Also a new version just came out, can you update to that?
SPEC: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/05698894-intel-media-driver-free/intel-media-driver-free.spec SRPM: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/05698894-intel-media-driver-free/intel-media-driver-free-23.1.4-1.fc39.src.rpm Rebased to the latest tag, MPEG-2 enabled.
SPEC: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/05700646-intel-media-driver-free/intel-media-driver-free.spec SRPM: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/05700646-intel-media-driver-free/intel-media-driver-free-23.1.4-1.fc39.src.rpm Added strip.py .
Copr build: https://copr.fedorainfracloud.org/coprs/build/5700638 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-1942132-intel-media-driver-free/fedora-rawhide-x86_64/05700638-intel-media-driver-free/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Created attachment 1953231 [details] The .spec file difference from Copr build 5700638 to 5700735
Copr build: https://copr.fedorainfracloud.org/coprs/build/5700735 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-1942132-intel-media-driver-free/fedora-rawhide-x86_64/05700735-intel-media-driver-free/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
SPEC: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/05705956-intel-media-driver-free/intel-media-driver-free.spec SRPM: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/05705956-intel-media-driver-free/intel-media-driver-free-23.1.4-1.fc39.src.rpm GEN10 iGPU support enabled.
Everything looks good to me now. I'm just waiting for FE-Legal approval and then I will approve this review.
Created attachment 1953430 [details] The .spec file difference from Copr build 5700735 to 5706040
Copr build: https://copr.fedorainfracloud.org/coprs/build/5706040 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-1942132-intel-media-driver-free/fedora-rawhide-x86_64/05706040-intel-media-driver-free/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
One last nitpick: ... -DVC1_Decode_Supported="no" \ -DAVC_Encode_VME_Supported="no" \ -DAVC_Encode_VDEnc_Supported="no" \ -DHEVC_Encode_VME_Supported="no" \ -DHEVC_Encode_VDEnc_Supported="no" \ -DAVC_Decode_Supported="no" \ -DHEVC_Decode_Supported="no" \ -DAVC_Decode_Supported="no" \ ... Could you sort these alphabetically? Also, could you post a message on legal mailing list to ask Fedora Legal folks about the status of their review of this code?
> Could you sort these alphabetically? Sure sure, will do when I get to next round of rebasing to a later upstream release next week. > Also, could you post a message on legal mailing list to ask Fedora Legal > folks about the status of their review of this code? I am not sure that'd help/is desired? There has been a discussion started a few months ago internally with our senior legal people.
(In reply to František Zatloukal from comment #53) > > Could you sort these alphabetically? > > Sure sure, will do when I get to next round of rebasing to a later upstream > release next week. Thanks. > > Also, could you post a message on legal mailing list to ask Fedora Legal > > folks about the status of their review of this code? > > I am not sure that'd help/is desired? There has been a discussion started a > few months ago internally with our senior legal people. No need, then. It's enough to know that evaluation is in progress.
František, can you please make the main binary package "libva-intel-media-driver"?
Why using the old vaapi naming scheme across the distro, for newer packages ? newer naming schema is more like having vaapi-driver in the name. Guideline is pretty clear on that the package name is derived from the source tarball! What justify that such guideline is arbitrary ignored ? Now one can add any virtual provides that make things fit!
What is the status of this task? Is over 5 months the usual pace for legal to review this? Or is there an actual need to move this to the ML to ensure the discussion and topic does not get buried? intel-media-driver-free is used for hardware-accelerated AV1 encode and decode, for example on Intel Arc DG2. OBS Studio nowadays supports AV1-based streaming to supported platforms (YouTube currently) as well as local recording and in the case of ffmpeg, the pieces have fallen into place and should be around in the next release of ffmpeg. Soon enough, this will be the bottleneck, and I'd really like to see Fedora remain a viable platform for video-oriented content creators.
I understand the only remaining concerns here are (a) debate over the package name, and (b) waiting for FE-Legal approval? Regarding (a), please folks, agree on what to name it. This is not a good reason for the package review to be stuck. :) Regarding (b), we cannot wait indefinitely on legal review. To block on legal, we need to have a clear deadline in place for when to proceed if FE-Legal does not object before then. I see the status in April was that legal review had begun "a few months ago" so it seems like there has been plenty of time to finish this task. I suggest we do not block on this any longer unless (a) there are specific concrete questions outstanding (I don't think there actually are), or (b) FE-Legal requests that we block (I don't think that it will). If we are genuinely unsure whether particular codecs implemented by this package are allowed in Fedora (I don't think we are), then just disable them. > I am not sure that'd help/is desired? There has been a discussion started a few months ago internally with our senior legal people. What was the outcome of this discussion?
I'm lifting FE-Legal on this bug. We have been waiting for over a year on it, and I firmly believe we've done sufficient diligence on this front. I've also filed a Fedora Council ticket about the problems with FE-Legal: https://pagure.io/Fedora-Council/tickets/issue/471 František, can you please implement my feedback as mentioned in comment 55? Once that's done and the driver is revved to the latest version released two weeks ago, I will approve this for inclusion.
SPEC: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/06724409-intel-media-driver-free/intel-media-driver-free.spec SRPM: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/06724409-intel-media-driver-free/intel-media-driver-free-23.4.2-1.fc40.src.rpm
The only thing I see is that the changelog entry needs to be fixed for today rather than several months ago, but that can be fixed on import. At this point, this looks as good as it gets, so.. PACKAGE APPROVED.
libigfxcmrt is still built as a separate shared object, but as such, this is a dead project: https://github.com/intel/cmrt Please remove this sub-package unless you can point to any users ? Also according to version numbers in 23.4 is pre-release software (might need any ABI breaking update). please only import in rawhide for now.
(In reply to Nicolas Chauvet (kwizart) from comment #62) > libigfxcmrt is still built as a separate shared object, but as such, this is > a dead project: https://github.com/intel/cmrt > > Please remove this sub-package unless you can point to any users ? I mean, I don't have any preference here, but what's the harm if it's in a subpackage and built by default? In my opinion, we can remove it once upstream at least starts not to build it by default? > > Also according to version numbers in 23.4 is pre-release software (might > need any ABI breaking update). please only import in rawhide for now. Yeah, I always forget that tags are just for development versions here, will keep that in mind once it's imported and follow stable releases unless we need to use devel for some reason. On the other hand, I don't see a big harm in importing it to f39 too? The API/ABI should be "libva/VAAPI" no? And if there arrives something that depends on the distributed .so (can/should that happen?), we can handle that in a side-tag?
(In reply to František Zatloukal from comment #63) > (In reply to Nicolas Chauvet (kwizart) from comment #62) > > libigfxcmrt is still built as a separate shared object, but as such, this is > > a dead project: https://github.com/intel/cmrt > > > > Please remove this sub-package unless you can point to any users ? > > I mean, I don't have any preference here, but what's the harm if it's in a > subpackage and built by default? In my opinion, we can remove it once > upstream at least starts not to build it by default? End-users will have to swap to the full intel-media-driver by hands. Having this package might interfere with this process. I don't get what you mean by "once it's upstream" ? There is no plan to get this library into a separate project. This is backward. cmrt is now part of intel-media-driver and not meant as separate library. > > Also according to version numbers in 23.4 is pre-release software (might > > need any ABI breaking update). please only import in rawhide for now. > > > Yeah, I always forget that tags are just for development versions here, will > keep that in mind once it's imported and follow stable releases unless we > need to use devel for some reason. Thanks > On the other hand, I don't see a big harm in importing it to f39 too? The > API/ABI should be "libva/VAAPI" no? And if there arrives something that > depends on the distributed .so (can/should that happen?), we can handle that > in a side-tag? What I mean by 23.4 is pre-release is that any later 23.4 serie can mandates any higher libva library (or else) than what currently has f39. Also Intel VAAPI is know to silent break ABI from time to time, so even minor releases need to be carefully audited. At the end, we don't want to rebuild all libva userspace deps for a single driver update (even if we could do this in fedora/rpmfusion it will nevertheless break many legitimate copr users that might have different maintenance pace). I hope to have clarified the point.
The Pagure repository was created at https://src.fedoraproject.org/rpms/intel-media-driver-free
Forgot to update this, the package is in the repositories, rawhide for now, f39 will come as soon as we get a stable 23.4.x release, if gmmlib and libva requirements aren't raised above f39 providers.