[Bug 1671064] New: Review Request: libldac - LDAC library from AOSP

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: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  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!

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

Gergely Gombos  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

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

Gergely Gombos  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |182235 (FE-Legal)

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=182235
[Bug 182235] Fedora Legal Tracker

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

Dominik 'Rathann' Mierzejewski  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
           Assignee|[e-mail]    |[e-mail]
              Flags|                            |fedora-review?

--- Comment #1 from Dominik 'Rathann' Mierzejewski  ---
I can take the review and sponsor you, assuming this passes legal review.

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

--- Comment #2 from Gergely Gombos  ---
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/

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

--- Comment #3 from Gergely Gombos  ---
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.

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

--- Comment #4 from Dominik 'Rathann' Mierzejewski  ---
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} \
     ./

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

--- Comment #5 from Gergely Gombos  ---
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

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

--- Comment #6 from Dominik 'Rathann' Mierzejewski  ---
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.

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

Tom "spot" Callaway  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
             Blocks|182235 (FE-Legal)           |

--- Comment #7 from Tom "spot" Callaway  ---
Lifting FE-Legal block, this package is fine.

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=182235
[Bug 182235] Fedora Legal Tracker

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

Dominik 'Rathann' Mierzejewski  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+

--- Comment #8 from Dominik 'Rathann' Mierzejewski  ---
Great, package is approved and I'm sponsoring Gergely into the packager group
now.

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

--- Comment #9 from Pasi Karkkainen  ---
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).

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

--- Comment #10 from Gwyn Ciesla  ---
(fedscm-admin):  The Pagure repository was created at
https://src.fedoraproject.org/rpms/libldac

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

Gergely Gombos  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

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

Gergely Gombos  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

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

--- Comment #11 from Dominik 'Rathann' Mierzejewski  ---
You can add

# big endian is not supported
https://bugzilla.redhat.com/show_bug.cgi?id=1677491
ExcludeArch: s390x

and build anyway for now.

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

--- Comment #12 from Gergely Gombos  ---
Thanks Dominik, I was just doing that. :)

* Fri Feb 15 2019 Gergely Gombos  - 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.

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

--- Comment #13 from Peter Robinson  ---
> 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.

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

Neal Gompa  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #14 from Neal Gompa  ---
(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

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |MODIFIED

--- Comment #15 from Fedora Update System  ---
libldac-2.0.2.2-4.fc29 has been submitted as an update to Fedora 29.
https://bodhi.fedoraproject.org/updates/FEDORA-2019-90c880c7cc

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

--- Comment #16 from Gergely Gombos  ---
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.

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|MODIFIED                    |ON_QA

--- Comment #17 from Fedora Update System  ---
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

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

https://bugzilla.redhat.com/show_bug.cgi?id=1671064

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ON_QA                       |CLOSED
         Resolution|---                         |ERRATA
        Last Closed|                            |2019-03-05 15:20:50

--- Comment #18 from Fedora Update System  ---
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.

[Bug 1671064] Review Request: libldac - LDAC library from AOSP

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