[Bug 2313902] New: Review Request: rocdecode - High-performance video decode SDK for AMD GPUs
https://bugzilla.redhat.com/show_bug.cgi?id=2313902
Bug ID: 2313902 Summary: Review Request: rocdecode - High-performance video decode SDK for AMD GPUs Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: alexjnewt@fastmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
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
https://bugzilla.redhat.com/show_bug.cgi?id=2313902
Jeremy Newton alexjnewt@fastmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |tflink@redhat.com, | |Tom.Rix@amd.com Doc Type|--- |If docs needed, set a value
https://bugzilla.redhat.com/show_bug.cgi?id=2313902
--- Comment #1 from Jeremy Newton alexjnewt@fastmail.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=2313902
Tom.Rix@amd.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |Tom.Rix@amd.com
https://bugzilla.redhat.com/show_bug.cgi?id=2313902
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://github.com/ROCm/roc | |Decode
--- Comment #2 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8053122 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2313902
Tom.Rix@amd.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(alexjnewt@fastmai | |l.com) Status|NEW |ASSIGNED
--- Comment #3 from Tom.Rix@amd.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=2313902
Jeremy Newton alexjnewt@fastmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(alexjnewt@fastmai | |l.com) |
--- Comment #4 from Jeremy Newton alexjnewt@fastmail.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=2313902
--- Comment #5 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2052750 --> https://bugzilla.redhat.com/attachment.cgi?id=2052750&action=edit The .spec file difference from Copr build 8053122 to 8157610
https://bugzilla.redhat.com/show_bug.cgi?id=2313902
--- Comment #6 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8157610 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2313902
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=2313902
Tom.Rix@amd.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+
--- Comment #7 from Tom.Rix@amd.com --- Looks good, thanks for the changes. Approved.
https://bugzilla.redhat.com/show_bug.cgi?id=2313902
Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |RELEASE_PENDING
--- Comment #8 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/rocdecode
https://bugzilla.redhat.com/show_bug.cgi?id=2313902
Jeremy Newton alexjnewt@fastmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RELEASE_PENDING |CLOSED Resolution|--- |RAWHIDE Last Closed| |2024-11-01 23:32:17
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com