Bug 1671064
Summary: | Review Request: libldac - LDAC library from AOSP | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Gergely Gombos <gombosg> |
Component: | Package Review | Assignee: | Dominik 'Rathann' Mierzejewski <dominik> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | dan, dominik, hannsj_uhl, heri, ngompa13, olysonek, package-review, pasik, pbrobinson, rabin, rsandu, samuel-rhbugs, sgraf, tcallawa, xavier |
Target Milestone: | --- | Flags: | dominik:
fedora-review+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | If docs needed, set a value | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2019-03-05 15:20:50 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | 1677491 | ||
Bug Blocks: |
Description
Gergely Gombos
2019-01-30 16:45:09 UTC
I can take the review and sponsor you, assuming this passes legal review. Thank you! For legal review: There's a NOTICE file in the source that says: https://android.googlesource.com/platform/external/libldac/+/master/NOTICE Taking the certification process is required to use LDAC in your products. For the detail of certification process, see the following URL: https://www.sony.net/Products/LDAC/aosp/ License tag is fine. Packaging-wise it looks very good, too. I have some nitpicks below: 1. I suggest listing pkgconfig and include files explicitly to avoid missing changes from upstream: --- libldac.spec.orig 2019-01-30 17:40:13.000000000 +0100 +++ libldac.spec 2019-02-07 18:50:43.172939850 +0100 @@ -50,8 +50,11 @@ %{_libdir}/libldacBT_enc.so.%{sonamebase}.* %files devel -%{_includedir}/* -%{_libdir}/pkgconfig/* +%dir %{_includedir}/ldac +%{_includedir}/ldac/ldacBT_abr.h +%{_includedir}/ldac/ldacBT.h +%{_libdir}/pkgconfig/ldacBT-abr.pc +%{_libdir}/pkgconfig/ldacBT-enc.pc %{_libdir}/libldacBT_abr.so %{_libdir}/libldacBT_enc.so 2. Summary: could be more informative, e.g. "A lossy audio codec for Bluetooth connections". 3. I'd also recommend using BR: cmake3 and %{cmake3} macro in %build to make it build seamlessly on EPEL as well. --- libldac.spec.orig 2019-01-30 17:40:13.000000000 +0100 +++ libldac.spec 2019-02-07 19:03:01.479784500 +0100 @@ -10,7 +10,7 @@ URL: https://github.com/EHfive/ldacBT Source0: %{url}/releases/download/v%{version}/%{archivename}-%{version}.tar.gz -BuildRequires: cmake +BuildRequires: cmake3 BuildRequires: gcc %package devel @@ -30,7 +30,7 @@ %autosetup -n %{archivename} %build -%{cmake} \ +%{cmake3} \ -DLDAC_SOFT_FLOAT=OFF \ -DINSTALL_LIBDIR=%{_libdir} \ ./ Thanks a lot for the suggestions, plus for directly including the patch snippets! I have applied them plus updated the description. Spec: https://pagure.io/libldac/raw/master/f/libldac.spec SRPM: https://pagure.io/libldac/raw/master/f/libldac-2.0.2.2-3.fc30.src.rpm Can you do at least one non-trivial unofficial package review while we're waiting for the FE-Legal block to be lifted (ping Tom if it doesn't happen soon)? Please pick something recent from https://fedoraproject.org/PackageReviewStatus/NEW.html This package is technically APPROVED. I'll change the review flag status once FE-Legal is lifted. Lifting FE-Legal block, this package is fine. Great, package is approved and I'm sponsoring Gergely into the packager group now. Great to see libldac getting approved in Fedora! Thanks a lot everyone. Btw are you interested in packaging libopenaptx (https://github.com/pali/libopenaptx) aswell? It's a separate LGPL library which implements only the AptX and AptX-HD codecs. I believe the patents around aptx expired last year.. It's possible to use libopenaptx instead of ffmpeg to get aptx/aptx-hd codecs supported in pulseaudio/bluetooth. (and there are patches for that on pulseaudio mailinglist already. patches to bluez were already merged). (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/libldac You can add # big endian is not supported https://bugzilla.redhat.com/show_bug.cgi?id=1677491 ExcludeArch: s390x and build anyway for now. Thanks Dominik, I was just doing that. :) * Fri Feb 15 2019 Gergely Gombos <gombosg> - 2.0.2.2-4 - Add s390x ExcludeArch See #1677491. I don't have an s390x machine nor an actual LDAC headset to patch & test upstream code so I'm excluding this arch. :) Builds fine now: Rawhide https://koji.fedoraproject.org/koji/taskinfo?taskID=32832242 F29 https://koji.fedoraproject.org/koji/taskinfo?taskID=32832365 This still fails for ppc64, with the same byte order error: F28 https://koji.fedoraproject.org/koji/taskinfo?taskID=32832369 (https://kojipkgs.fedoraproject.org//work/tasks/2399/32832399/build.log) Does anyone know if there is a difference between F28 ppc64 and F29 ppc64 byte order support? I either have to exclude ppc64 or not release for F28, right? Which one is the better choice in this case? (I'd rather not release for F28 since ppc64 works on F29+ and F28 would be EOL'd anyway soon.) Thanks for the help. > Does anyone know if there is a difference between F28 ppc64 and F29 ppc64 > byte order support? ppc64 is big endian, ppc64le is little endian. > I either have to exclude ppc64 or not release for F28, right? Which one is > the better choice in this case? I just wouldn't release for F-28, new functionality should really only be going to the newer releases. (In reply to Pasi Karkkainen from comment #9) > Great to see libldac getting approved in Fedora! Thanks a lot everyone. > > Btw are you interested in packaging libopenaptx > (https://github.com/pali/libopenaptx) aswell? It's a separate LGPL library > which implements only the AptX and AptX-HD codecs. I believe the patents > around aptx expired last year.. > > It's possible to use libopenaptx instead of ffmpeg to get aptx/aptx-hd > codecs supported in pulseaudio/bluetooth. > (and there are patches for that on pulseaudio mailinglist already. patches > to bluez were already merged). Feel free to propose a package review request for it, and it can be looked at to bring into the distribution. See here: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers And here: https://fedoraproject.org/wiki/Package_Review_Process libldac-2.0.2.2-4.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-90c880c7cc Wrapping up my first package's release! I decided not to release this for F28 due to ppc64 (big-endian) failing. From F29+ apparently pp64le is default, which works. F29 build: https://koji.fedoraproject.org/koji/buildinfo?buildID=1210898 Rawhide build: https://koji.fedoraproject.org/koji/buildinfo?buildID=1210896 F29 update: https://bodhi.fedoraproject.org/updates/FEDORA-2019-90c880c7cc This still depends on the s390x exclusion bug (https://bugzilla.redhat.com/show_bug.cgi?id=1677491), but there's not much to do since the upstream code throws an error on big-endian systems. Let me know if some further actions are needed. libldac-2.0.2.2-4.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-90c880c7cc libldac-2.0.2.2-4.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report. |