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

Benson Muite benson_muite@emailplus.org changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |benson_muite@emailplus.org Assignee|nobody@fedoraproject.org |benson_muite@emailplus.org Flags| |fedora-review? Doc Type|--- |If docs needed, set a value

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

Neal Gompa ngompa13@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |ngompa13@gmail.com

--- Comment #2 from Neal Gompa ngompa13@gmail.com ---

# Use old cmake macro behaviour. %define __cmake_in_source_build 1

Why are you doing this?

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

--- Comment #4 from Neal Gompa ngompa13@gmail.com ---

%cmake . \

You need to drop "." too.

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

--- Comment #6 from Benson Muite benson_muite@emailplus.org --- Warnings from fedora-review

[!]: Large data in /usr/share should live in a noarch subpackage if package is arched. Note: Arch-ed rpms have a total of 29736960 bytes in /usr/share mediastreamer2-devel-5.2.45-1.fc39.x86_64.rpm:29245440 See:

https://fedoraproject.org/wiki/Packaging:ReviewGuidelines#Package_Review_Gui...

Maybe some of these files should be in a noarch data subpackage?

- If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. Note: License file mediastreamer2_license.html is not marked as %license See: https://docs.fedoraproject.org/en-US/packaging- guidelines/LicensingGuidelines/#_license_text

Other license found in the source: No copyright* SGI Free Software License B v2.0 ----------------------------------------------- mediastreamer2-5.2.45/include/OpenGL/GLES2/gl2platform.h mediastreamer2-5.2.45/include/OpenGL/GLES3/gl3platform.h

Apache License 2.0 ------------------ mediastreamer2-5.2.45/src/android/AudioTrack.h mediastreamer2-5.2.45/src/android/media/NdkMediaCodec.h mediastreamer2-5.2.45/src/android/media/NdkMediaCrypto.h mediastreamer2-5.2.45/src/android/media/NdkMediaDrm.h mediastreamer2-5.2.45/src/android/media/NdkMediaError.h mediastreamer2-5.2.45/src/android/media/NdkMediaExtractor.h mediastreamer2-5.2.45/src/android/media/NdkMediaFormat.h mediastreamer2-5.2.45/src/android/media/NdkMediaMuxer.h

Apple MIT License ----------------- mediastreamer2-5.2.45/src/utils/opengl_debug.h

BSD 3-Clause License -------------------- mediastreamer2-5.2.45/include/OpenGL/LICENSE mediastreamer2-5.2.45/src/utils/_kiss_fft_guts.h mediastreamer2-5.2.45/src/utils/kiss_fft.c mediastreamer2-5.2.45/src/utils/kiss_fftr.c

BSD 3-Clause License GNU Affero General Public License v3.0 or later -------------------------------------------------------------------- mediastreamer2-5.2.45/include/mediastreamer2/dsptools.h mediastreamer2-5.2.45/src/audiofilters/macsnd.c mediastreamer2-5.2.45/src/utils/dsptools.c

BSD 3-Clause License GNU General Public License v2.0 or later ------------------------------------------------------------- mediastreamer2-5.2.45/src/audiofilters/aqsnd.m

Please add a license breakdown in the spec file.

Kiss FFT is available in Fedora. If the one packaged in Fedora cannot be used, should indicate that it uses a bundled Kiss FFT.

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

--- Comment #8 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5940668 (succeeded)

Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...

Please take a look if any issues were found.

--- 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=2208737

--- Comment #10 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5940673 (succeeded)

Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...

Please take a look if any issues were found.

--- 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=2208737

--- Comment #12 from Phil Wyett philip.wyett@kathenas.org --- Additional:

Fixed and added patch to rectify doxygen creating .html files with bad links. INSTALL file remove as no longer exists.

Will be reported upstream in due course.

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

--- Comment #14 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5945814 (succeeded)

Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...

Please take a look if any issues were found.

--- 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=2208737

--- Comment #16 from Phil Wyett philip.wyett@kathenas.org --- Spec URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/medi... SRPM URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/medi...

(In reply to Benson Muite from comment #15)

Thanks. Can you also create man pages using doxygen?

Sadly not possible currently. Will encourage upstream to have man in the future.

Why exclude the file %{_libdir}/pkgconfig/mediastreamer.pc

Have had issues with pkgconfig files from upstream, but now testing is complete I have removed the exclusion and the file is included.

Is it possible to remove include/OpenGL before the build so it is not installed and ensure headers from Fedora packages are used?

I want upstream to move away from inclusion, so have done it the way I have done it and added patch: 0006_mediastreamer2_do_not_use_bundled_opengl_headers.patch so that the folder is never used in builds.

I will approach them also about detecting and using system kiss-fft.

Additional: Have removed conditional around "BuildRequires: pkgconfig(libyuv)" as it is now included in EPEL for EL builds in the near future.

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

--- Comment #18 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5970652 (succeeded)

Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...

Please take a look if any issues were found.

--- 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=2208737

--- Comment #20 from Phil Wyett philip.wyett@kathenas.org --- Spec URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/medi... SRPM URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/medi...

(In reply to Benson Muite from comment #19)

Thanks. It should generate man pages:

https://gitlab.linphone.org/BC/public/mediastreamer2/-/blob/master/help/ Doxyfile.in#L1826

Maybe a patch is needed for the CMakeLists file: https://gitlab.linphone.org/BC/public/mediastreamer2/-/blob/master/help/ CMakeLists.txt

There are a variety of issues with manpage generation and all will be addressed with upstream in time and are not required to be fixed as part of a review.

I will be trying to build a relationship up with upstream and have changes made good for all; also improve overall quality. Upstream has no easy way to report bugs for all packages related to Linphone is via one Mailing List (ML) it seems and that is already not ideal. :-/

Based on what you have removed licenses needed seem to be BSD 3-Clause License GNU General Public License v2.0 or later GNU Affero General Public License v3.0 or later

Done.

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

--- Comment #22 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5971784 (succeeded)

Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...

Please take a look if any issues were found.

--- 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.

Product: Fedora Version: rawhide Component: Package Review

Benson Muite benson_muite@emailplus.org has canceled Package Review package-review@lists.fedoraproject.org's request for Benson Muite benson_muite@emailplus.org's needinfo: Bug 2208737: Review Request: mediastreamer2 - Audio/Video real-time streaming https://bugzilla.redhat.com/show_bug.cgi?id=2208737

--- Comment #24 from Benson Muite benson_muite@emailplus.org --- [fedora-review-service-build]