Bug 2267321 - Review Request: rtmpdump - Toolkit for RTMP streams
Summary: Review Request: rtmpdump - Toolkit for RTMP streams
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL: https://rtmpdump.mplayerhq.hu/
Whiteboard:
Depends On:
Blocks: MultimediaSIG
TreeView+ depends on / blocked
 
Reported: 2024-03-01 15:31 UTC by Dominik 'Rathann' Mierzejewski
Modified: 2024-03-19 14:33 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2024-03-19 14:33:44 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
The .spec file difference from Copr build 7095301 to 7106479 (1.18 KB, patch)
2024-03-05 09:02 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 7106479 to 7106653 (757 bytes, patch)
2024-03-05 09:41 UTC, Fedora Review Service
no flags Details | Diff

Description Dominik 'Rathann' Mierzejewski 2024-03-01 15:31:38 UTC
Spec URL: https://rathann.fedorapeople.org/review/rtmpdump/rtmpdump.spec
SRPM URL: https://rathann.fedorapeople.org/review/rtmpdump/rtmpdump-2.4-25.20240228gitb7c7976.fc41.src.rpm
Description:
rtmpdump is a toolkit for RTMP streams. All forms of RTMP are supported,
including rtmp://, rtmpt://, rtmpe://, rtmpte://, and rtmps://.

Fedora Account System Username: rathann

Comment 1 Fedora Review Service 2024-03-01 15:36:25 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/7095301
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2267321-rtmpdump/fedora-rawhide-x86_64/07095301-rtmpdump/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 2 Neal Gompa 2024-03-01 16:37:37 UTC
> Version:        2.4
> Release:        25.%{gitdate}git%{shortcommit}%{?dist}

Please switch to snapshot versioning in the Version field.

Cf. https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots

Comment 3 Xavier Bachelot 2024-03-01 16:53:57 UTC
> %install
> make CRYPTO=GNUTLS DESTDIR=%{buildroot} prefix=%{_prefix} mandir=%{_mandir} libdir=%{_libdir} install

Please use %make_install and split the long line

Comment 4 Dominik 'Rathann' Mierzejewski 2024-03-01 20:58:04 UTC
(In reply to Neal Gompa from comment #2)
> > Version:        2.4
> > Release:        25.%{gitdate}git%{shortcommit}%{?dist}
> 
> Please switch to snapshot versioning in the Version field.
> 
> Cf.
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/
> #_snapshots

No. This is a post-release snapshot and upstream has just announced the intention to release 2.6:
https://lists.mplayerhq.hu/pipermail/rtmpdump/2024-March/002554.html

Comment 5 Dominik 'Rathann' Mierzejewski 2024-03-01 20:58:33 UTC
(In reply to Xavier Bachelot from comment #3)
> > %install
> > make CRYPTO=GNUTLS DESTDIR=%{buildroot} prefix=%{_prefix} mandir=%{_mandir} libdir=%{_libdir} install
> 
> Please use %make_install and split the long line

OK. Anything else?

Comment 6 Dominik 'Rathann' Mierzejewski 2024-03-05 08:56:40 UTC
(In reply to Dominik 'Rathann' Mierzejewski from comment #4)
> (In reply to Neal Gompa from comment #2)
> > > Version:        2.4
> > > Release:        25.%{gitdate}git%{shortcommit}%{?dist}
> > 
> > Please switch to snapshot versioning in the Version field.
> > 
> > Cf.
> > https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/
> > #_snapshots
> 
> No. This is a post-release snapshot and upstream has just announced the
> intention to release 2.6:
> https://lists.mplayerhq.hu/pipermail/rtmpdump/2024-March/002554.html

On second thought, OK.

Comment 7 Dominik 'Rathann' Mierzejewski 2024-03-05 08:57:26 UTC
(In reply to Xavier Bachelot from comment #3)
> > %install
> > make CRYPTO=GNUTLS DESTDIR=%{buildroot} prefix=%{_prefix} mandir=%{_mandir} libdir=%{_libdir} install
> 
> Please use %make_install and split the long line

Done. It's no longer so long after the change. :)

Comment 8 Dominik 'Rathann' Mierzejewski 2024-03-05 08:58:43 UTC
Spec URL: https://rathann.fedorapeople.org/review/rtmpdump/rtmpdump.spec
SRPM URL: https://rathann.fedorapeople.org/review/rtmpdump/rtmpdump-2.4%5e20240228gitb7c7976-1.fc41.src.rpm

* Tue Mar 05 2024 Dominik Mierzejewski <dominik> - 2.4^20240228gitb7c7976-1
- use recommended snapshot versioning
- use modern macros

Comment 9 Fedora Review Service 2024-03-05 09:02:23 UTC
Created attachment 2020126 [details]
The .spec file difference from Copr build 7095301 to 7106479

Comment 10 Fedora Review Service 2024-03-05 09:02:25 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/7106479
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2267321-rtmpdump/fedora-rawhide-x86_64/07106479-rtmpdump/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 Dominik 'Rathann' Mierzejewski 2024-03-05 09:36:04 UTC
[fedora-review-service-build]

- ensure Fedora CFLAGS and LDFLAGS are used consistently

Comment 12 Fedora Review Service 2024-03-05 09:41:43 UTC
Created attachment 2020134 [details]
The .spec file difference from Copr build 7106479 to 7106653

Comment 13 Fedora Review Service 2024-03-05 09:41:45 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/7106653
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2267321-rtmpdump/fedora-rawhide-x86_64/07106653-rtmpdump/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 14 Xavier Bachelot 2024-03-13 18:54:02 UTC
Maybe missing an explicit BR: make rather than relying on deps ?
Otherwise looks good, will try and do a full review asap.

Comment 15 Keith Diaz 2024-03-14 06:31:35 UTC
(In reply to Fedora Review Service from comment #13)
> Copr build:
> https://copr.fedorainfracloud.org/coprs/build/7106653
> (succeeded)
> 
> Review template:
> https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-
> review-2267321-rtmpdump/fedora-rawhide-x86_64/07106653-rtmpdump/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 https://fnafgames.io
> 
> If you want to trigger a new Copr build, add a comment containing new
> Spec and SRPM URLs or [fedora-review-service-build] string.

Thanks for the info

Comment 16 Fedora Review Service 2024-03-14 06:47:50 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/7158010
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2267321-rtmpdump/fedora-rawhide-x86_64/07158010-rtmpdump/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 17 Xavier Bachelot 2024-03-14 10:36:12 UTC
Could you please take a look at the following rpmlint warnings ?
librtmp.x86_64: W: crypto-policy-non-compliance-gnutls-2 /usr/lib64/librtmp.so.1 gnutls_priority_init
librtmp.x86_64: W: crypto-policy-non-compliance-gnutls-1 /usr/lib64/librtmp.so.1 gnutls_priority_set_direct

See https://docs.fedoraproject.org/en-US/packaging-guidelines/CryptoPolicies/

And also :
librtmp.x86_64: W: binary-or-shlib-calls-gethostbyname /usr/lib64/librtmp.so.1

Both shall probably at least be notified to upstream.

Comment 18 Dominik 'Rathann' Mierzejewski 2024-03-19 11:38:04 UTC
(In reply to Xavier Bachelot from comment #14)
> Maybe missing an explicit BR: make rather than relying on deps ?

Good point, will add when importing.

> Otherwise looks good, will try and do a full review asap.

Thanks!

(In reply to Xavier Bachelot from comment #17)
> Could you please take a look at the following rpmlint warnings ?
> librtmp.x86_64: W: crypto-policy-non-compliance-gnutls-2
> /usr/lib64/librtmp.so.1 gnutls_priority_init
> librtmp.x86_64: W: crypto-policy-non-compliance-gnutls-1
> /usr/lib64/librtmp.so.1 gnutls_priority_set_direct
> 
> See https://docs.fedoraproject.org/en-US/packaging-guidelines/CryptoPolicies/
> 
> And also :
> librtmp.x86_64: W: binary-or-shlib-calls-gethostbyname
> /usr/lib64/librtmp.so.1
> 
> Both shall probably at least be notified to upstream.

They are valid and I sent them upstream. However, I wonder if RTMP
even supports IPv6 or if there are specific old crypto
requirements at work here.

Comment 19 Dominik 'Rathann' Mierzejewski 2024-03-19 14:33:44 UTC
rtmpdump upstream advice is not to include it in Fedora at all.

Only two RPM Fusion packages depend on rtmpdump: get-flash-videos and qarte.

First is dead (last commit 4 years ago) and the second is specific to one website (arte.tv).

So... probably not worth packaging. Withdrawing review request.


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