https://bugzilla.redhat.com/show_bug.cgi?id=1671064
Bug ID: 1671064 Summary: Review Request: libldac - LDAC library from AOSP Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: gombosg@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec: https://pagure.io/libldac/raw/master/f/libldac.spec SRPM: https://pagure.io/libldac/raw/master/f/libldac-2.0.2.2-2.fc30.src.rpm
Description: LDAC library from AOSP. LDAC is an audio coding technology developed by Sony. It enables the transmission of High-Resolution Audio content, even over a Bluetooth connection.
Fedora Account System Username: gombosg
I originally submitted to RPMFusion: https://bugzilla.rpmfusion.org/show_bug.cgi?id=5154
This package may be suitable for the Fedora repos depending on if and how LDAC itself is patented. The library itself has an Apache v2 license.
This is my first RPM package. I'm looking for a sponsor because of this!
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
Gergely Gombos gombosg@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- External Bug ID| |RPM Fusion 5154 Blocks| |177841 (FE-NEEDSPONSOR)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
Gergely Gombos gombosg@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |182235 (FE-Legal)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=182235 [Bug 182235] Fedora Legal Tracker
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
Dominik 'Rathann' Mierzejewski dominik@greysector.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |dominik@greysector.net Assignee|nobody@fedoraproject.org |dominik@greysector.net Flags| |fedora-review?
--- Comment #1 from Dominik 'Rathann' Mierzejewski dominik@greysector.net --- I can take the review and sponsor you, assuming this passes legal review.
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
--- Comment #2 from Gergely Gombos gombosg@gmail.com --- 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/
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
--- Comment #3 from Gergely Gombos gombosg@gmail.com --- Hi, I have gotten the following reply from Tom Callaway: "The LDAC implementation in libldac might be patented by Sony (the inventor of the codec), but because Sony is also the author of libldac and they put that code under Apache 2.0, which includes an explicit patent grant... It is fine for Fedora. Because there was some odd wording in the NOTICE file accompanying the libldac source, I had to confirm with Red Hat Legal that we were okay to ignore it and include libldac as Apache 2.0 without that requirement, but they gave me the okay."
Hopefully he'll confirm it here as his time permits, but I think it is safe to start to reviewing process.
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
--- Comment #4 from Dominik 'Rathann' Mierzejewski dominik@greysector.net --- 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} \ ./
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
--- Comment #5 from Gergely Gombos gombosg@gmail.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
--- Comment #6 from Dominik 'Rathann' Mierzejewski dominik@greysector.net --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
Tom "spot" Callaway tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |tcallawa@redhat.com Blocks|182235 (FE-Legal) |
--- Comment #7 from Tom "spot" Callaway tcallawa@redhat.com --- Lifting FE-Legal block, this package is fine.
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=182235 [Bug 182235] Fedora Legal Tracker
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
Dominik 'Rathann' Mierzejewski dominik@greysector.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #8 from Dominik 'Rathann' Mierzejewski dominik@greysector.net --- Great, package is approved and I'm sponsoring Gergely into the packager group now.
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
--- Comment #9 from Pasi Karkkainen pasik@iki.fi --- 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).
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
--- Comment #10 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/libldac
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
Gergely Gombos gombosg@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |1677491
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1677491 [Bug 1677491] libldac doesn't support big-endian, s390x build fails
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
Gergely Gombos gombosg@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841 (FE-NEEDSPONSOR) |
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
--- Comment #11 from Dominik 'Rathann' Mierzejewski dominik@greysector.net --- You can add
# big endian is not supported https://bugzilla.redhat.com/show_bug.cgi?id=1677491 ExcludeArch: s390x
and build anyway for now.
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
--- Comment #12 from Gergely Gombos gombosg@gmail.com --- Thanks Dominik, I was just doing that. :)
* Fri Feb 15 2019 Gergely Gombos gombosg@gmail.com - 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.
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
--- Comment #13 from Peter Robinson pbrobinson@gmail.com ---
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.
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |ngompa13@gmail.com
--- Comment #14 from Neal Gompa ngompa13@gmail.com --- (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
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
--- Comment #15 from Fedora Update System updates@fedoraproject.org --- libldac-2.0.2.2-4.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-90c880c7cc
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
--- Comment #16 from Gergely Gombos gombosg@gmail.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #17 from Fedora Update System updates@fedoraproject.org --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=1671064
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed| |2019-03-05 15:20:50
--- Comment #18 from Fedora Update System updates@fedoraproject.org --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=1671064 Bug 1671064 depends on bug 1677491, which changed state.
Bug 1677491 Summary: libldac doesn't support big-endian, s390x build fails https://bugzilla.redhat.com/show_bug.cgi?id=1677491
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |EOL
package-review@lists.fedoraproject.org