Spec URL: https://mystro256.fedorapeople.org/rocdecode.spec SRPM URL: https://mystro256.fedorapeople.org/rocdecode-6.2.0-1.fc42.src.rpm COPR Build: https://copr.fedorainfracloud.org/coprs/mystro256/playground/build/8046290/ Description: rocDecode is a high-performance video decode SDK for AMD GPUs. Using the rocDecode API, you can access the video decoding features available on your GPU. Fedora Account System Username: mystro256
Question, there's some video samples included in the devel package. Should I put this in its own samples subpackage? Or just through the video files in a data subpackage? Not sure. Also I'm aware of the non-SPDX license field. I'll update it when we figure out where ever the samples should go.
Copr build: https://copr.fedorainfracloud.org/coprs/build/8053122 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2313902-rocdecode/fedora-rawhide-x86_64/08053122-rocdecode/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - Not a valid SPDX expression 'MIT and BSD'. It seems that you are using the old Fedora license abbreviations. Try `license-fedora2spdx' for converting it to SPDX. Read more: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1 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.
No gcc is fine, the checker is not smart enough to figure out dependencies of hip's cc. The SPDX check is nagging about 'and' it prefers 'AND' What files were BSD? The license file is not boilerplate MIT, can you include the non MIT part under the %license field ? This part: NOTICE REGARDING STANDARDS AMD does not provide a license or sublicense to any Intellectual Property Rights relating to any standards, including but not limited to any audio and/or video codec technologies such as MPEG-2, MPEG-4; AVC/H.264; HEVC/H.265; AAC decode/FFMPEG; AAC encode/FFMPEG; VC-1; and MP3 (collectively, the “Media Technologies”). For clarity, you will pay any royalties due for such third party technologies, which may include the Media Technologies that are owed as a result of AMD providing the Software to you. For the patch to deal with llvm/bin/clang++, could this be done with a sed line in %prep ? Other rocm packages have --with debug and --with test options. Please add these and if they are not mockable, note why. ex/ downloads stuff.. I like the idea of having samples, but they need to work, these do not. If it is too much effort to get them in shape, drop including them.
Updated: Spec URL: https://mystro256.fedorapeople.org/rocdecode.spec SRPM URL: https://mystro256.fedorapeople.org/rocdecode-6.2.2-1.fc42.src.rpm COPR Build: https://copr.fedorainfracloud.org/coprs/mystro256/playground/build/8157598/ > What files were BSD? Not sure why I put that there. Deleted. > The license file is not boilerplate MIT, can you include the non MIT part under the %license field ? Well I put a comment above "License:", is this what you had in mind? As for %license, I did include the license file in question. As per the guidelines, the license field isn't legally binding, hence the need for the license macro and license file from source. The clause also doesn't really affect the MIT licensing, it's more of a clarification/reminder that the user doesn't get the codecs for free when using this software. > could this be done with a sed line in %prep ? Done, also made a PR for the patch upstream, so it can hopefully be dropped in the near future. > Other rocm packages have --with debug and --with test options. So I just put the "with test" condition in there. The tests don't actually work due to some cmake logic bugginess, but I suspect it actually requires ffmpeg-freeworld from rpmfusion for some tests to work correctly anyway. I'll leave it disabled for now with a note. > If it is too much effort to get them in shape, drop including them. The samples appear to be used in the tests. I'm unsure how valuable they are, so I just dropped them for now.
Created attachment 2052750 [details] The .spec file difference from Copr build 8053122 to 8157610
Copr build: https://copr.fedorainfracloud.org/coprs/build/8157610 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2313902-rocdecode/fedora-rawhide-x86_64/08157610-rocdecode/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ 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.
Looks good, thanks for the changes. Approved.
The Pagure repository was created at https://src.fedoraproject.org/rpms/rocdecode