Bug 2316282 - Review Request: svt-av1-psy - SVT-AV1 with perceptual enhancements optimal AV1 encoding
Summary: Review Request: svt-av1-psy - SVT-AV1 with perceptual enhancements optimal ...
Keywords:
Status: CLOSED EOL
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: 40
Hardware: All
OS: Linux
unspecified
low
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/gianni-rosato/svt-...
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2024-10-03 12:50 UTC by solomoncyj
Modified: 2025-06-04 05:35 UTC (History)
7 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2025-05-20 19:57:55 UTC
Type: ---
Embargoed:
solomoncyj: needinfo-


Attachments (Terms of Use)
The .spec file difference from Copr build 8105376 to 8145377 (2.88 KB, patch)
2024-10-16 01:34 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 8145377 to 8230681 (3.54 KB, patch)
2024-11-08 06:42 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 8230681 to 8697904 (3.93 KB, patch)
2025-02-25 08:14 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 8697904 to 8718523 (7.19 KB, patch)
2025-03-03 07:57 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 8718523 to 8718608 (434 bytes, patch)
2025-03-03 08:27 UTC, Fedora Review Service
no flags Details | Diff

Description solomoncyj 2024-10-03 12:50:52 UTC
Spec URL: https://github.com/solomoncyj/svt-av1-psy/releases/download/v2.2.1-A/svt-av1-psy.spec
SRPM URL: https://github.com/solomoncyj/svt-av1-psy/releases/download/v2.2.1-A/svt-av1-psy-2.2.1.A-1.fc41.src.rpm
Description: SVT-AV1  with perceptual enhancements  optimal AV1 encoding
Fedora Account System Username: solomoncyj


Reproducible: Couldn't Reproduce

Comment 1 Fedora Review Service 2024-10-03 13:01:22 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8105376
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2316282-svt-av1-psy/fedora-rawhide-x86_64/08105376-svt-av1-psy/fedora-review/review.txt

Found issues:

- Not a valid SPDX expression 'BSD-3-Clause-Clear AND BSD-2-Clause'.
  Read more: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1
- Documentation size is 4876478 bytes in 232 files. 
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation

Please know that there can be false-positives.

---
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.

Comment 2 Cristian Le 2024-10-03 13:35:54 UTC
- Please add `-DUSE_EXTERNAL_CPUINFO=ON` and delete `third_party/cpuinfo`
- Delete all `third_party` actually. We need to go over those files to make sure there are no unpackageable files that we need to sanitize first
- Enable the unit tests (`-DBUILD_TESTING=ON`)
- The `HAVE_ARM_PLATFORM` section will be very tricky to handle for `aarch64` builders (all of the `ENABLE_NEON` and similar options). I don't have the expertise there, maybe you can ping this in devel to get a second pair of eyes.
- I think `EXCLUDE_HASH` (and by extension `REPRODUCIBLE_BUILDS`) should be `ON`
- Unconditional `add_subdirectory(third_party/fastfeat)` is problematic :(. It's a dead project though, so maybe you can just use `Provides: bundled(fastfeat)` in this case.
- I need to go through the build 3-4 more times, there are so many more weird stuff to review in there.

(Oh god I hate the build system for this project as well)

Comment 3 Richard Fontana 2024-10-03 13:58:24 UTC
Regarding the license issue, see: https://gitlab.com/fedora/legal/fedora-license-data/-/issues/383
https://gitlab.com/fedora/legal/fedora-license-data/-/blob/main/data/LicenseRef-BSD-3-Clause-Clear-WITH-AdditionRef-AOMPL-1.0.toml?ref_type=heads

We might want to change this to get rid of the `LicenseRef-`, but maybe it's better not to.

Comment 4 Richard Fontana 2024-10-03 13:59:22 UTC
Removing the FE-Legal block.

Comment 5 Cristian Le 2024-10-03 14:02:37 UTC
The license issue is with regards to the patents, not the BSD-3-Clause-Clear tag. Are the patents used there acceptable?

Comment 6 Richard Fontana 2024-10-03 14:12:55 UTC
(In reply to Cristian Le from comment #5)
> The license issue is with regards to the patents, not the BSD-3-Clause-Clear
> tag. Are the patents used there acceptable?

The reason why BSD-3-Clause-Clear (better known to some of us as ClearBSD) is not allowed (by default) in Fedora is that it excludes the grant of a patent license. In this case, what we have is a supplementing of that license with a patent license in the PATENTS.md file.  The previous conclusion was basically that the combination of the copyright license and the patent license was acceptable as an open source license. 

If you think there is a patent issue that is not satisfactorily covered under the license in the PATENTS file -- for example (this is completely hypothetical, to be clear) you have specific reason to believe that the package infringes patent X and you have specific reason to believe the license in the PATENTS file doesn't extend to patent X -- we can look into that.

Comment 7 solomoncyj 2024-10-04 05:30:23 UTC
(In reply to Cristian Le from comment #2)
> - Please add `-DUSE_EXTERNAL_CPUINFO=ON` and delete `third_party/cpuinfo`
> - Delete all `third_party` actually. We need to go over those files to make
> sure there are no unpackageable files that we need to sanitize first
> - Enable the unit tests (`-DBUILD_TESTING=ON`)
> - The `HAVE_ARM_PLATFORM` section will be very tricky to handle for
> `aarch64` builders (all of the `ENABLE_NEON` and similar options). I don't
> have the expertise there, maybe you can ping this in devel to get a second
> pair of eyes.
> - I think `EXCLUDE_HASH` (and by extension `REPRODUCIBLE_BUILDS`) should be
> `ON`
> - Unconditional `add_subdirectory(third_party/fastfeat)` is problematic :(.
> It's a dead project though, so maybe you can just use `Provides:
> bundled(fastfeat)` in this case.
> - I need to go through the build 3-4 more times, there are so many more
> weird stuff to review in there.
> 
> (Oh god I hate the build system for this project as well)

request 2 blocks building. requesting legal team to take a look

Comment 8 Cristian Le 2024-10-04 09:07:31 UTC
Similar to [1] it seems there are more complicated license breakdowns. Hopefully licensecheck can help navigate through them.

Breakdown of the `third_party` stuff:
- `aom/inc/aom`: Used only in tests. Should be available in Fedora through `aom` [2]. Delete if running the tests proves too hard, otherwise indicate in the SourceLicense breakdown or comments.
- `aom_dsp`: Same as above, but upstream does not seem to expose it.
- `cpuinfo`: Available in fedora build supports linking to it. Please delete it and use `-DUSE_EXTERNAL_CPUINFO=ON`.
- `fastfeat`: Dead project and there seems to be an alternative code-generated version [3]. For now though, just `Provides: bundled()` it and include it's license.
- `googletest`: Available in Fedora. Needs some patching to remove the `third_party/googletest` references and replace them with `find_package(GTest)`, `target_link_libraries(GTest::gtest GTest::gtest_main)` and a few other stuff. Alternatively open an issue and ask them to support linking against system `gtest` (or better have `FetchContent(FIND_PACKAGE_ARGS)` support).
- `safestringlib`: Available upstream project [4] but not packaged to Fedora. Please open an issue in either project to request supporting that.

In any case the license breakdown should indicate the `third_party` stuff, either inside the `License` field or `SourceLicense` field (for the case of `googletest` which is not packaged), and appropriate `Provides: bundled()` should be added.

[1]: https://src.fedoraproject.org/rpms/svt-av1/blob/rawhide/f/svt-av1.spec
[2]: https://src.fedoraproject.org/rpms/aom
[3]: https://github.com/edrosten/fast-C-src/issues/3
[4]: https://github.com/intel/safestringlib

Comment 9 solomoncyj 2024-10-06 11:54:10 UTC
i will be dropping the tests. when i did it, the tests timed out and failed, stopping the builds. case in point : https://copr.fedorainfracloud.org/coprs/solomoncyj/SVT-AV1-PSY/build/8111629/

Comment 10 solomoncyj 2024-10-06 11:58:01 UTC
This would mean removing gtest, gmock, aom, libaom-devel from the project

Comment 11 Cristian Le 2024-10-15 12:32:57 UTC
For this packages, let's just go with `Provides: bundled()` since de-bundeling the rest is rather tricky and the issues should also be coordinated with `svt-av1` upstream. I.e. keep `safestringlib` as is, disable tests similar to `svt-av1`, but still link to the external `cpuinfo` since they seem to already support it.

After maybe 1-2 rounds of review, I'll ask Neal Gompa to also take a look at it or bounce it around a bit since the package is almost identical with `svt-av1` and there would be some coordination needed between those, and to check if they are compatible with one another.

Comment 13 Cristian Le 2024-10-15 14:11:29 UTC
So far only small changes remain:
- `BSD-3-Clause-Clear` -> `LicenseRef-BSD-3-Clause-Clear-WITH-AdditionRef-AOMPL-1.0`
- See the license breakdown in upstream [1], those licenses should also be included
- `valgrind-devel` is a development tool like a linter, it should not be used
- The `ENABLE_AVX512` option is actually wrongly defined in the CMakeLists.txt, but also supporting a subset of architectures is more complicated, I would need some guidance on this subject as well on how to review this.
- For now move `SVT_AV1_PGO` to a TODO? The implication of it seem quite complicated, and maybe it would be good to investigate what it does and how much of an impact it has
- Despite upstream documentation, `BUILD_DEC` is not really used anywhere, can remove it
- I would recommend adding a few TODOs on things that seem interesting to investigate like: `ENABLE_AVX512`, `SVT_AV1_PGO`, `REPRODUCIBLE_BUILDS` + `EXCLUDE_HASH`
  (you can find some of them by running `ccmake -B build`, but beware it is not always comprehensive)
- Use a `bcond_check` and wrap the `%check` section to ignore it for now, and comment about the failing tests that need to be investigated
- Does `svt-av1`/`svt-av1-psy` work without decoder parts? Maybe some additional `Provides` would be in order? Need someone more familiar with the whole project to comment on this part

[1]: https://src.fedoraproject.org/rpms/svt-av1/blob/rawhide/f/svt-av1.spec#_12-19

Comment 14 solomoncyj 2024-10-15 14:18:31 UTC
they do work without decoders, as dav1d is the preffered decoder nowadays

Comment 16 Fedora Review Service 2024-10-16 01:34:55 UTC
Created attachment 2052224 [details]
The .spec file difference from Copr build 8105376 to 8145377

Comment 17 Fedora Review Service 2024-10-16 01:34:58 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8145377
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2316282-svt-av1-psy/fedora-rawhide-x86_64/08145377-svt-av1-psy/fedora-review/review.txt

Found issues:

- Documentation size is 4876987 bytes in 232 files. 
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation

Please know that there can be false-positives.

---
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.

Comment 18 Cristian Le 2024-10-16 10:12:45 UTC
I went through the `Source` folder for each file, although `licensecheck` also helped with the breakdown:
- `Source/API/*`, `Source/App/*`, `Source/Lib/**` except the ones below: LicenseRef-BSD-2-Clause-WITH-AdditionRef-AOMPL-1.0
- `Source/API/EbSvtAv1Enc.h`, `Source/App/app_config.c`, `Source/App/app_output_ivf.c`, `Source/App/app_output_ivf.h`, `Source/Lib/ASM_AVX2/mc.h`, `Source/Lib/Codec/coding_loop.c`, `Source/Lib/Codec/enc_cdef.c`, `Source/Lib/Codec/enc_dec_process.c`, `Source/Lib/Codec/full_loop.c`, `Source/Lib/Codec/initial_rc_process.c`, `Source/Lib/Codec/inter_prediction.c`, `Source/Lib/Codec/mode_decision.c`, `Source/Lib/Codec/pass2_strategy.c`, `Source/Lib/Codec/pd_process.c`, `Source/Lib/Codec/pic_analysis_process.c`, `Source/Lib/Codec/product_coding_loop.c`, `Source/Lib/Codec/temporal_filtering.c`, `Source/Lib/Globals/enc_handle.c`, `Source/Lib/Globals/enc_settings.c`, `Source/Lib/Globals/metadata_handle.h`: LicenseRef-BSD-3-Clause-Clear-WITH-AdditionRef-AOMPL-1.0
- `Source/Lib/ASM_AVX2/dav1d_x86inc.asm`, `Source/Lib/ASM_SSE2/x86inc.asm`: ISC
- `Source/Lib/ASM_AVX2/itx*`, `Source/Lib/ASM_AVX2/mc*.asm`, `Source/Lib/ASM_NEON/*.S`, `Source/Lib/ASM_NEON/itx_lbd_neon.h`: BSD-2-Clause
- `Source/Lib/Codec/vector.*`: MIT
- `third_party/safestringlib`: MIT
- `third_party/fastfeat`: BSD-3-Clause

Maybe you can write the comment as main library is a tangled mixture of `LicenseRef-BSD-2-Clause-WITH-AdditionRef-AOMPL-1.0` and `LicenseRef-BSD-3-Clause-Clear-WITH-AdditionRef-AOMPL-1.0` in order to avoid the long breakdown of the latter.

The `License` section should therefore be: `LicenseRef-BSD-2-Clause-WITH-AdditionRef-AOMPL-1.0 AND LicenseRef-BSD-3-Clause-Clear-WITH-AdditionRef-AOMPL-1.0 AND ISC AND BSD-2-Clause AND BSD-3-Clause AND MIT` (maybe format it with `%{shrink:` to make it human readable)

Other than that all looks fine and I will ping Neal after that for second opinion and to see if we can get you sponsored.

Comment 19 Cristian Le 2024-10-16 11:28:18 UTC
Oh, I missed a few stuff:
- `Provides: bundled` part, get rid of the quotation marks "
- The review is complaining about the `Docs` being too big. You could split it and put in a `docs` sub project. `svt-av1` is putting it in the `devel-docs` for some reason, but reading the contents it seems there are some config file documentation in there also
- There are conflicting auto-generated `Provides: pkgconfig(SvtAv1Enc)` in `svt-av1-psy-devel`. Not sure what are the procedures to handle this

Comment 22 Cristian Le 2024-10-18 11:07:52 UTC
LGTM, just make the SPEC and SRPM URL comment so that others can more easily run `fedora-review` on it. After that I will try to needinfo someone who is more familiar with `svt-av1`

Comment 23 Simone Caronni 2024-10-21 18:05:49 UTC
(In reply to Cristian Le from comment #19)
> - There are conflicting auto-generated `Provides: pkgconfig(SvtAv1Enc)` in
> `svt-av1-psy-devel`. Not sure what are the procedures to handle this

This should definitely disappear unless the plan is to make this the replacement of vanilla svt-av1. Maybe change the code to provide pkgconfig(SvtAv1Enc-psy)?

Comment 24 solomoncyj 2024-10-22 00:45:02 UTC
(In reply to Simone Caronni from comment #23)
> (In reply to Cristian Le from comment #19)
> > - There are conflicting auto-generated `Provides: pkgconfig(SvtAv1Enc)` in
> > `svt-av1-psy-devel`. Not sure what are the procedures to handle this
> 
> This should definitely disappear unless the plan is to make this the
> replacement of vanilla svt-av1. Maybe change the code to provide
> pkgconfig(SvtAv1Enc-psy)?

this can be use as a drop in for vanilla svt-av1. it id the same just without the svtAv1Dec

Comment 25 Cristian Le 2024-10-22 13:17:35 UTC
The libraries may be a drop in replacement, but the provides issue is a bit different. Consider if there is a package `foo.spec` which has the line
```
BuildRequires:   pkgconfig(SvtAv1Enc)
```
Then which project would it be built against. Similarly in that `foo.spec` you do not specify a `Requires` because that is automatically detected from checking the linkages of the binary and libraries, but then which package will it automatically populate the `Requires` field with? I know there are examples with binary packages like different versions of `mariadb` and there was a talk on that design in the last devconf.cz, but for the library case and something that will be injected in `BuildRequires` I don't know what are the implications.

I do not have enough experience with this issue so we should get more feedback on this issue and what needs to be done.

Comment 26 solomoncyj 2024-10-23 15:47:03 UTC
https://github.com/gianni-rosato/svt-av1-psy/issues/85#event-14818727310 only problem with renaming: it will break the "drop in" effect. how about removing the pkgconf: for the psy and have the users explicitly use the devel package?

Comment 28 solomoncyj 2024-11-08 06:32:17 UTC
programs will be built aginst the og lib, how ever users can swap the libary at runtime to use this lib

Comment 29 Fedora Review Service 2024-11-08 06:42:29 UTC
Created attachment 2056454 [details]
The .spec file difference from Copr build 8145377 to 8230681

Comment 30 Fedora Review Service 2024-11-08 06:42:31 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8230681
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2316282-svt-av1-psy/fedora-rawhide-x86_64/08230681-svt-av1-psy/fedora-review/review.txt

Found issues:

- Documentation size is 4764605 bytes in 232 files. 
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation

Please know that there can be false-positives.

---
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.

Comment 31 Cristian Le 2024-11-08 12:57:37 UTC
The only comment I have left is that the `Conflicts` should be with `svt-av1-libs`. But at this point the only issue is how to make it compatible with `svt-av1` since they have the same SONAME/SOVERSION libraries. I don't have experience with this so I hope someone else can chime in on this issue.

Pinging Neal and Robert as maintainers for `svt-av1` if they can help here.

Comment 32 solomoncyj 2024-11-27 07:48:45 UTC
https://giannirosato.com/blog/post/end-of-psy/

Comment 33 solomoncyj 2025-02-25 07:16:15 UTC
apprently the project is being mantained by other people now

Comment 35 Fedora Review Service 2025-02-25 08:14:59 UTC
Created attachment 2077737 [details]
The .spec file difference from Copr build 8230681 to 8697904

Comment 36 Fedora Review Service 2025-02-25 08:15:01 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8697904
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2316282-svt-av1-psy/fedora-rawhide-x86_64/08697904-svt-av1-psy/fedora-review/review.txt

Found issues:

- Documentation size is 4765831 bytes in 232 files. 
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation

Please know that there can be false-positives.

---
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.

Comment 37 solomoncyj 2025-02-25 08:24:32 UTC
 abidiff --no-harmful --redundant --verbose /home/solomoncyj/Downloads/svt-av1-libs-2.3.0-1.fc41.x86_64/usr/lib64/libSvtAv1Enc.so.2.3.0 /home/solomoncyj/Downloads/review-svt-av1-psy/rpms-unpacked/svt-av1-psy-2.3.0.B-1.20250225gitv2.3.0.b.fc42.x86_64.rpm/usr/lib64/libSvtAv1Enc.so.2.3.0
building die -> parent maps ... DONE@/home/solomoncyj/Downloads/svt-av1-libs-2.3.0-1.fc41.x86_64/usr/lib64/libSvtAv1Enc.so.2.3.0:0s6ms
DWARF Reader: building the libabigail internal representation ...
DWARF Reader: building the libabigail internal representation DONE for corpus /home/solomoncyj/Downloads/svt-av1-libs-2.3.0-1.fc41.x86_64/usr/lib64/libSvtAv1Enc.so.2.3.0 in: 0s258ms
DWARF Reader: Number of aggregate types compared: 319
Number of canonical types propagated: 149
Number of cancelled propagated canonical types:0
Number of suppressed functions: 11782
Number of allowed functions: 3200
Total number of fns in the corpus: 23
Total number of variables in the corpus: 0
DWARF Reader: resolving declaration only classes ... DONE@/home/solomoncyj/Downloads/svt-av1-libs-2.3.0-1.fc41.x86_64/usr/lib64/libSvtAv1Enc.so.2.3.0 in :0s
resolving declaration only enums ... DONE@/home/solomoncyj/Downloads/svt-av1-libs-2.3.0-1.fc41.x86_64/usr/lib64/libSvtAv1Enc.so.2.3.0:0s
DWARF Reader: fixing up functions with linkage name but no advertised underlying symbols ....0 functions to fixup, potentially
 DONE@/home/solomoncyj/Downloads/svt-av1-libs-2.3.0-1.fc41.x86_64/usr/lib64/libSvtAv1Enc.so.2.3.0 in :0s
DWARF Reader: perform late type canonicalizing ...
DWARF Reader is going to canonicalize 16169 types from corpus /home/solomoncyj/Downloads/svt-av1-libs-2.3.0-1.fc41.x86_64/usr/lib64/libSvtAv1Enc.so.2.3.0
sorting types before canonicalization ... 
sorted types for c14n in: 0s81ms

hashing types before c14n ...
hashed types in: 0s164ms

Canonicalizing types ...
Canonicalizing of types DONE in: 0s41ms

DWARF Reader finished types sorting, hashing & canonicalizing in: 0s287ms
DWARF Reader: late type canonicalizing DONE for /home/solomoncyj/Downloads/svt-av1-libs-2.3.0-1.fc41.x86_64/usr/lib64/libSvtAv1Enc.so.2.3.0 in :0s287ms
DWARF Reader: sort functions and variables ... DONE@/home/solomoncyj/Downloads/svt-av1-libs-2.3.0-1.fc41.x86_64/usr/lib64/libSvtAv1Enc.so.2.3.0:0s 
Compute diff ...
diff computed!:0s
Computing net changes ...
Applying suppressions ...
suppressions applied!:0s
Marking leaf nodes ...
leaf nodes marked!:0s
Applying filters and computing diff stats ...
in apply_filters_and_compute_diff_stats:applying filters to 0 changed fns ...
in apply_filters_and_compute_diff_stats:filters to changed fn applied!:0s
in apply_filters_and_compute_diff_stats:applying filters to 0 changed vars ...
in apply_filters_and_compute_diff_stats:filters to changed vars applied!:0s
in apply_filters_and_compute_diff_stats:applying filters to unreachable types ...
in apply_filters_and_compute_diff_stats:filters to unreachable types applied!:0s
in apply_filters_and_compute_diff_stats:categorizing redundant changed sub nodes ...
in apply_filters_and_compute_diff_stats:redundant changed sub nodes categorized!:0s
in apply_filters_and_compute_diff_stats:count changed fns ...
in apply_filters_and_compute_diff_stats:changed fn counted!:0s
in apply_filters_and_compute_diff_stats:count changed vars ...
in apply_filters_and_compute_diff_stats:changed vars counted!:0s
in apply_filters_and_compute_diff_stats:count leaf changed types ...
in apply_filters_and_compute_diff_stats:changed leaf types counted!:0s
in apply_filters_and_compute_diff_stats:count leaf changed artefacts ...
in apply_filters_and_compute_diff_stats:changed leaf artefacts counted!:0s
in apply_filters_and_compute_diff_stats:count unreachable types ...
in apply_filters_and_compute_diff_stats:unreachable types counted!:0s
Filters applied and diff stats computed!: 0s
net changes computed!: 0s
Computing incompatible changes ...
Computing changes ...
changes computed!: 0s
Computing report ...
Functions changes summary: 0 Removed, 0 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
Function symbols changes summary: 0 Removed, 1 Added function symbol not referenced by debug info
Variable symbols changes summary: 0 Removed, 0 Added variable symbol not referenced by debug info

1 Added function symbol not referenced by debug info:

  [A] svt_psy_get_version

Report computed!:0s

Comment 38 Robert-André Mauchin 🐧 2025-02-27 17:48:35 UTC
Ok my idea, since I have worked on it to, was to use this package as a drop-in replacement, since this is the same library.

So provides exactly the same as the original one, that the user could dnf swap it.

If you put the lib inside the main package it will conflict with the original libs package too and the swap won't be easy.

What I would change:

%bcond_with check

-> 

%bcond check 1

to have consistent syntax.


# cpuinfo is only available for %%{x86_64} %%{arm64} on Fedora

->

# cpuinfo and libdovi are only availables for %%{x86_64} %%{arm64} on Fedora


%if %{with libdovi}
BuildRequires:  libdovi-devel
%endif

->

%if %{with libdovi}
BuildRequires:  pkgconfig(dovi)
%endif

-> Prefer to use pkgconfig() and cmake() in your spec


Something is wrong here:

%if %{with unbundled_cpuinfo}
BuildRequires:  pkgconfig(libcpuinfo)


Provides: bundled(cpuinfo) = 0^20201129gita7e1076-1
%endif


if you unbundle, you should not have Provides: bundled(cpuinfo) at the same time:

%if %{with unbundled_cpuinfo}
BuildRequires:  pkgconfig(libcpuinfo)
%endif

and

%if %{without unbundled_cpuinfo}
Provides: bundled(cpuinfo) = 0^20201129gita7e1076-1
%endif

Regarding :

Version:         2.3.0.B

->

Version:        2.3.0~B

with

%global version_with_dash %(echo %{version} | tr '~' '-')

and 

Source:         %url/archive/v%{version_with_dash}/SVT-AV1-%{version_with_dash}.tar.gz

But if you use forge :

%global tag v%(echo %{version} | tr '~' '-')



Regarding the conflicts, the guidelines are
https://docs.fedoraproject.org/en-US/packaging-guidelines/Conflicts/#_library_name_conflicts

which suggests: https://docs.fedoraproject.org/en-US/packaging-guidelines/EnvironmentModules/

and https://docs.fedoraproject.org/en-US/packaging-guidelines/Alternatives/

But Ihave not much experience with this.

Comment 39 Robert-André Mauchin 🐧 2025-02-27 17:50:38 UTC
This is my current version if you want inspiration:

%bcond check 1

# cpuinfo and libdovi is only available for %%{x86_64} %%{arm64} on Fedora
%ifarch %{x86_64} %{arm64}
%bcond unbundled_cpuinfo 1
%bcond libdovi 1
%endif

%global version_with_dash %(echo %{version} | tr '~' '-')

%global _description %{expand:
SVT-AV1-PSY is the Scalable Video Technology for AV1 (SVT-AV1 Encoder and
Decoder) with perceptual enhancements for psychovisually optimal AV1 encoding.
The goal is to create the best encoding implementation for perceptual quality
with AV1.}

Name:           svt-av1-psy
Version:        2.3.0~B
Release:        %autorelease
Summary:        SVT-AV1 with perceptual enhancements for optimal AV1 encoding

# Main library: BSD-3-Clause-Clear and AOMPL
# https://gitlab.com/fedora/legal/fedora-license-data/-/issues/383
# Source/Lib/Common/Codec/EbHmCode.c: BSD
# Source/App/EncApp/EbAppString.*
# Source/Lib/Common/Codec/EbString.*
# Source/Lib/Encoder/Codec/vector.*: MIT
# Source/Lib/Common/ASM_SSE2/x86inc.asm: ISC
# Source/App/DecApp/EbMD5Utility.*: PublicDomain
# third_party/fastfeat : BSD-3-Clause
# third_party/safestringlib : MIT
License:        LicenseRef-BSD-3-Clause-Clear-WITH-AdditionRef-AOMPL-1.0 AND MIT AND ISC AND LicenseRef-Fedora-Public-Domain
URL:            https://github.com/psy-ex/svt-av1-psy
Source:         %url/archive/v%{version_with_dash}/SVT-AV1-%{version_with_dash}.tar.gz

BuildRequires:  cmake >= 3.16
BuildRequires:  gcc >= 5.4.0
BuildRequires:  gcc-c++ >= 5.4.0
BuildRequires:  help2man
BuildRequires:  nasm >= 2.14
BuildRequires:  ninja-build
%if %{with libdovi}
BuildRequires:  pkgconfig(dovi)
%endif
%if %{with unbundled_cpuinfo}
BuildRequires:  pkgconfig(libcpuinfo)
%endif

Requires:       %{name}-libs%{?_isa} = %{?epoch:%{epoch}:}%{version}-%{release}

%description %_description

%package    libs
Summary:    SVT-AV1-PSY libraries
%if ! %{with gst_plugin}
Obsoletes:  gstreamer1-%{name} < %{?epoch:%{epoch}:}%{version}-%{release}
%endif

%if %{without unbundled_cpuinfo}
Provides: bundled(cpuinfo) = 0^20201129gita7e1076-1
%endif
Provides: bundled(fastfeat) = 0^20191113git391d5e9-1
Provides: bundled(safestringlib) = 1.0.0-1

%description libs %_description

This package contains SVT-AV1-PSY libraries.

%package    devel
Summary:    Development files for SVT-AV1-PSY
Requires:   %{name}-libs = %{?epoch:%{epoch}:}%{version}-%{release}
Recommends: %{name}-devel-docs = %{?epoch:%{epoch}:}%{version}-%{release}

%description devel %_description.

This package contains the development files for SVT-AV1-PSY.

%package    devel-docs
Summary:    Development documentation for SVT-AV1-PSY
BuildArch:  noarch

%description devel-docs %_description.

This package contains the documentation for development of SVT-AV1-PSY.


%prep
%autosetup -p1 -n %{name}-%{version_with_dash}

# Mitigate name collisions
mv third_party/fastfeat/LICENSE LICENSE.fastfeat
mv third_party/safestringlib/LICENSE LICENSE.safestringlib

# Sanitize third_party
%if %{with unbundled_cpuinfo}
rm -rfv third_party/cpuinfo
%endif
rm -rfv third_party/aom*
rm -rfv third_party/googletest


%build
%cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo \
       -DCMAKE_INSTALL_LIBDIR=%{_libdir} \
%if %{with unbundled_cpuinfo}
       -DUSE_EXTERNAL_CPUINFO=ON \
%else
       -DUSE_EXTERNAL_CPUINFO=OFF \
%endif
%if %{with libdovi}
       -DLIBDOVI_FOUND=1 \
%endif
       -DSVT_AV1_LTO=ON \
       -DSVT_AV1_PGO=ON \
       -G Ninja \
%cmake_build


%install
%cmake_install
rm -rfv %{buildroot}%{_libdir}/*.{a,la}

install -dm0755 %{buildroot}/%{_mandir}/man1
export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:%{buildroot}%{_libdir}
help2man -N --help-option=--help --no-discard-stderr --version-string=%{version} %{buildroot}%{_bindir}/SvtAv1EncApp > %{buildroot}%{_mandir}/man1/SvtAv1EncApp.1


%if %{with check}
%check
%ctest
%endif


%files
%{_bindir}/SvtAv1EncApp
%{_mandir}/man1/SvtAv1EncApp.1*

%files libs
%license LICENSE.md PATENTS.md LICENSE.fastfeat LICENSE.safestringlib
%doc CHANGELOG.md CONTRIBUTING.md README.md
%{_libdir}/libSvtAv1Enc.so.2*

%files devel
%{_includedir}/svt-av1
%{_libdir}/libSvtAv1Enc.so
%{_libdir}/pkgconfig/SvtAv1Enc.pc

%files devel-docs
%license LICENSE.md PATENTS.md LICENSE.fastfeat LICENSE.safestringlib
%doc Docs

%changelog
%autochangelog

Comment 41 Fedora Review Service 2025-03-03 07:57:57 UTC
Created attachment 2078643 [details]
The .spec file difference from Copr build 8697904 to 8718523

Comment 42 Fedora Review Service 2025-03-03 07:57:59 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8718523
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2316282-svt-av1-psy/fedora-rawhide-x86_64/08718523-svt-av1-psy/fedora-review/review.txt

Found issues:

- Documentation size is 4767205 bytes in 233 files. 
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation

Please know that there can be false-positives.

---
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.

Comment 44 Fedora Review Service 2025-03-03 08:27:00 UTC
Created attachment 2078644 [details]
The .spec file difference from Copr build 8718523 to 8718608

Comment 45 Fedora Review Service 2025-03-03 08:27:02 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8718608
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2316282-svt-av1-psy/fedora-rawhide-x86_64/08718608-svt-av1-psy/fedora-review/review.txt

Found issues:

- Documentation size is 4767205 bytes in 233 files. 
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation

Please know that there can be false-positives.

---
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.

Comment 47 Fedora Review Service 2025-03-24 10:07:11 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8811844
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2316282-svt-av1-psy/fedora-rawhide-x86_64/08811844-svt-av1-psy/fedora-review/review.txt

Found issues:

- svt-av1-psy.spec.spec should be svt-av1-psy.spec 
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_file_naming
- Documentation size is 4767205 bytes in 233 files. 
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation

Please know that there can be false-positives.

---
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.

Comment 48 Robert-André Mauchin 🐧 2025-03-28 17:05:10 UTC
Let's get back on this as I need it and I accidentally wiped by review folder

Ok gstreamer1-plugins-bad-free needs to be fixed to use pkgconfig(SvtAv1Enc), otherwise it is failing when we swap the package. The whole thing would benefit to use pkgconfig.

The SPEC looks good, it is working locally so far.

It builds on Mock.

Now we have a second problem. The assumption was svt-av1 and svt-av1-psy have the same soname. Now we have official svt-av1 at SONAME 3 and SVT-AV1-PSY at SONAME 2.
Considering SVT-AV1-PSY does not have a lot of (wo|man)power, we will have a problem to build it and swap it if the main soname is bigger than the one by svt-av1-psy.

It also means each time we soname bump the main package, we will break the psy package and break the upgrade path.

Comment 49 Aoife Moloney 2025-04-28 13:55:43 UTC
This message is a reminder that Fedora Linux 40 is nearing its end of life.
Fedora will stop maintaining and issuing updates for Fedora Linux 40 on 2025-05-13.
It is Fedora's policy to close all bug reports from releases that are no longer
maintained. At that time this bug will be closed as EOL if it remains open with a
'version' of '40'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, change the 'version' 
to a later Fedora Linux version. Note that the version field may be hidden.
Click the "Show advanced fields" button if you do not see it.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora Linux 40 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora Linux, you are encouraged to change the 'version' to a later version
prior to this bug being closed.

Comment 50 Aoife Moloney 2025-05-20 19:57:55 UTC
Fedora Linux 40 entered end-of-life (EOL) status on 2025-05-13.

Fedora Linux 40 is no longer maintained, which means that it
will not receive any further security or bug fix updates. As a result we
are closing this bug.

If you can reproduce this bug against a currently maintained version of Fedora Linux
please feel free to reopen this bug against that version. Note that the version
field may be hidden. Click the "Show advanced fields" button if you do not see
the version field.

If you are unable to reopen this bug, please file a new report against an
active release.

Thank you for reporting this bug and we are sorry it could not be fixed.


Note You need to log in before you can comment on or make changes to this bug.