Bug 2208737 - Review Request: mediastreamer2 - Audio/Video real-time streaming
Summary: Review Request: mediastreamer2 - Audio/Video real-time streaming
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Benson Muite
QA Contact: Fedora Extras Quality Assurance
URL: https://linphone.org/technical-corner...
Whiteboard:
Depends On:
Blocks: MultimediaSIG
TreeView+ depends on / blocked
 
Reported: 2023-05-20 05:00 UTC by Phil Wyett
Modified: 2023-08-23 16:30 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:
benson_muite: fedora-review?


Attachments (Terms of Use)
The .spec file difference from Copr build 5940674 to 5945814 (2.81 KB, patch)
2023-05-23 07:20 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 5945814 to 5970652 (892 bytes, patch)
2023-05-28 10:43 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 5970652 to 5971784 (572 bytes, patch)
2023-05-29 04:18 UTC, Fedora Review Service
no flags Details | Diff

Description Phil Wyett 2023-05-20 05:00:03 UTC
Spec URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/mediastreamer2/mediastreamer2.spec
SRPM URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/mediastreamer2/mediastreamer2-5.2.45-1.fc39.src.rpm

Description:

Mediastreamer2 is a library to make audio and video real-time
streaming and processing. It is written in pure C and based upon the
oRTP library.

Fedora Account System Username: kathenas

Additional: Part of the Linphone stack.

Comment 2 Neal Gompa 2023-05-21 08:19:06 UTC
> # Use old cmake macro behaviour.
> %define __cmake_in_source_build 1

Why are you doing this?

Comment 3 Phil Wyett 2023-05-21 08:38:56 UTC
(In reply to Neal Gompa from comment #2)
> > # Use old cmake macro behaviour.
> > %define __cmake_in_source_build 1
> 
> Why are you doing this?

This was in error and not required. Rectified and new upload of .spec and .src.rpm done.

Spec URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/mediastreamer2/mediastreamer2.spec
SRPM URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/mediastreamer2/mediastreamer2-5.2.45-1.fc39.src.rpm

Comment 4 Neal Gompa 2023-05-21 09:23:00 UTC
> %cmake . \

You need to drop "." too.

Comment 5 Phil Wyett 2023-05-21 09:27:34 UTC
(In reply to Neal Gompa from comment #4)
> > %cmake . \
> 
> You need to drop "." too.

Yes. Was just above to upload. Done now.

Spec URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/mediastreamer2/mediastreamer2.spec
SRPM URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/mediastreamer2/mediastreamer2-5.2.45-1.fc39.src.rpm

Comment 6 Benson Muite 2023-05-21 15:46:08 UTC
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_Guidelines

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.

Comment 7 Fedora Review Service 2023-05-22 06:22:28 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5940640
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2208737-mediastreamer2/fedora-rawhide-x86_64/05940640-mediastreamer2/fedora-review/review.txt

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.

Comment 8 Fedora Review Service 2023-05-22 06:36:26 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5940668
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2208737-mediastreamer2/fedora-rawhide-x86_64/05940668-mediastreamer2/fedora-review/review.txt

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.

Comment 9 Fedora Review Service 2023-05-22 06:39:42 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5940674
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2208737-mediastreamer2/fedora-rawhide-x86_64/05940674-mediastreamer2/fedora-review/review.txt

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.

Comment 10 Fedora Review Service 2023-05-22 06:41:15 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5940673
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2208737-mediastreamer2/fedora-rawhide-x86_64/05940673-mediastreamer2/fedora-review/review.txt

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.

Comment 11 Phil Wyett 2023-05-23 06:54:18 UTC
Spec URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/mediastreamer2/mediastreamer2.spec
SRPM URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/mediastreamer2/mediastreamer2-5.2.45-1.fc39.src.rpm

(In reply to Benson Muite from comment #6)
> 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_Guidelines
> 
> Maybe some of these files should be in a noarch data subpackage?

Split tester application off into own sub-package and created a noarch tester-data sub-package.

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

Done. Added to license field. Will do a deep dive here later and report to upstream.

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

Done, indicated as bundled.

This is a partial of 'kiss-fft'. Will report upstream and see if we can make system 'kiss-fft' usable.

Regards

Phil

Comment 12 Phil Wyett 2023-05-23 06:59:11 UTC
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.

Comment 13 Fedora Review Service 2023-05-23 07:20:44 UTC
Created attachment 1966373 [details]
The .spec file difference from Copr build 5940674 to 5945814

Comment 14 Fedora Review Service 2023-05-23 07:20:45 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5945814
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2208737-mediastreamer2/fedora-rawhide-x86_64/05945814-mediastreamer2/fedora-review/review.txt

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.

Comment 15 Benson Muite 2023-05-28 09:34:06 UTC
Thanks. Can you also create man pages using doxygen?

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

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

Comment 16 Phil Wyett 2023-05-28 10:16:05 UTC
Spec URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/mediastreamer2/mediastreamer2.spec
SRPM URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/mediastreamer2/mediastreamer2-5.2.45-1.fc39.src.rpm

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

Comment 17 Fedora Review Service 2023-05-28 10:43:03 UTC
Created attachment 1967537 [details]
The .spec file difference from Copr build 5945814 to 5970652

Comment 18 Fedora Review Service 2023-05-28 10:43:05 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5970652
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2208737-mediastreamer2/fedora-rawhide-x86_64/05970652-mediastreamer2/fedora-review/review.txt

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.

Comment 19 Benson Muite 2023-05-28 14:57:32 UTC
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

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

See output of license check:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2208737-mediastreamer2/fedora-rawhide-x86_64/05945814-mediastreamer2/fedora-review/licensecheck.txt

Comment 20 Phil Wyett 2023-05-29 03:51:56 UTC
Spec URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/mediastreamer2/mediastreamer2.spec
SRPM URL: https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/mediastreamer2/mediastreamer2-5.2.45-1.fc39.src.rpm

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

Comment 21 Fedora Review Service 2023-05-29 04:18:54 UTC
Created attachment 1967604 [details]
The .spec file difference from Copr build 5970652 to 5971784

Comment 22 Fedora Review Service 2023-05-29 04:18:56 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5971784
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2208737-mediastreamer2/fedora-rawhide-x86_64/05971784-mediastreamer2/fedora-review/review.txt

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.


Note You need to log in before you can comment on or make changes to this bug.