> autoreconf -ivf
move to %build

> PKG_CHECK_MODULES(ORTP, ortp >= 0.21.0,[found_ortp=true],foo=bar)
> PKG_CHECK_MODULES(MEDIASTREAMER, mediastreamer >= 2.8.99,[found_ms2=true],foo=bar)
add BuildRequires: pkgconfig(ortp) and pkgconfig(mediastreamer) if those are in fedora and you think we should enable those features (don't know what those packages do, so it's up to you to check them and think if we should enable them)

> # Fix FSF addresses
don't do that, just report bug in upstream and note link somewhere around License tag.

> License:       GPLv2
it's GPLv2+ actually

> find %{buildroot} -name '*.la' -exec rm {} \;
find %{buildroot}%{_libdir} -type f -name '*.la' -delete -print

Other things:
* replace bcg729 with %{name} in %files section
* Add spawning test-suite in %check

>> # Fix FSF addresses
> don't do that, just report bug in upstream and note link somewhere around License tag.
Any particular reason? In the past, I've been instructed to fix the FSF addr. From my understanding, it also makes sense - or does this violate "shipping a license file different from upstream" in you opinion? Now whether this needs to be done with seds or with a huge patch is another issue, but plenty of packages are doing this.

why not ?

find %{buildroot} -name '*.la' -delete

Concering FSF address:
https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address

So COPYING should not be patched, rest can but does not have to be.

(In reply to Sandro Mani from comment #4)
> Concering FSF address:
> https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address
> 
> So COPYING should not be patched, rest can but does not have to be.
so don't waste CPU cycles on doing sed, just report bug in upstream and forget about it until next release ;)

- Test not enabled since it downloads data from the internet, but this data yelds a checksum failure
- Enabling mediastreamer results in the library built as a mediastreamer plugin, which is what is desired for twinkle

Rest done:
Spec URL: https://smani.fedorapeople.org/review/bcg729.spec
SRPM URL: https://smani.fedorapeople.org/review/bcg729-1.0.0-2.fc25.src.rpm

%changelog
* Wed Jul 20 2016 Sandro Mani <manisandro> - 1.0.0-2
- Move autoreconf to build
- BR: pkgconfig(ortp)
- License is GPLv2+
- Change command to delete *.la files
- Use %%name in %%files
- Don't fix FSF addresses

> - Enabling mediastreamer results in the library built as a mediastreamer plugin, which is what is desired for twinkle
Unfortunately I don't understand this..

* add --disable-silent-rules to %configure
* %{name} is not used everywhere in %files
* file bug to upstream that AC_PROG_LIBTOOL is obsoleted and they should use LT_INIT
* file bug to upstream about incorrect FSF address

Nothing else to complain about.

> * add --disable-silent-rules to %configure
> * %{name} is not used everywhere in %files

Done

> * file bug to upstream that AC_PROG_LIBTOOL is obsoleted and they should use LT_INIT
> * file bug to upstream about incorrect FSF address
Sent to linphone-developers, should appear at [1] some time.

Thanks for the review!

[1] http://lists.nongnu.org/archive/html/linphone-developers/2016-07/index.html

(Followup remark: orpt is only used when building the mediastreamer plugin, hence removed again)

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/bcg729

bcg729-1.0.0-2.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-2c219c2a4d

bcg729-1.0.0-2.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-8e02805a7a

Has the patent for Annex A of ITU-T G.729 finally expired?

Did anyone run this by legal to make sure there's no issues with what's described at:
https://fedoraproject.org/wiki/Software_Patents

bcg729-1.0.0-2.fc23 has been pushed to the Fedora 23 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-2016-2c219c2a4d

bcg729-1.0.0-2.fc24 has been pushed to the Fedora 24 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-2016-8e02805a7a

I fear I was completely unaware of the patent situation surrounding this codec, and missed the corresponding line in the README file:

"Licensing: The source code is licensed under GPLv2. ITU G729 usage is governed by a patent license to be acquired from Sipro Lab"

I can't find any hints that the patent situation has changed, in which case this package needs to be removed from the distribution. Apologies for missing this, multimedia stuff always requires special attention...

I mailed spot [1], if he invite us to remove this package from fedora then We send it to rpmfusion.

[1] - https://fedoraproject.org/wiki/User:Spot

Uh, yeah. This cannot go into Fedora. Sorry.

would you like to send it for inclusion in rpmfusion ?

Submitted for review as RFBZ #4155. Closing this bug.

(In reply to Itamar Reis Peixoto from comment #17)
> I mailed spot [1], if he invite us to remove this package from fedora then
> We send it to rpmfusion.
> 
> [1] - https://fedoraproject.org/wiki/User:Spot

(In reply to Tom "spot" Callaway from comment #18)
> Uh, yeah. This cannot go into Fedora. Sorry.

I'd like to reopen discussion about the validity of 3rd party claims about patent ownership covering some VoIP codec algorithms related to this codec.

Upstream of this library believes that any patents covering most widely used A/B annexes of the codec have just expired (nov 2016).

https://github.com/BelledonneCommunications/bcg729/commit/2f6b033

Tom, could you please ask Fedora Legal to add this on their radar?

Actually all G.72x codecs have a very lengthy history dating back to late 80ies - early 90ies, so chances are high that some of them are no longer covered by unexpired patents.

Does bcg729 implement Annex C+, D, E, F, G, H, or I?

(In reply to Tom "spot" Callaway from comment #22)
> Does bcg729 implement Annex C+, D, E, F, G, H, or I?

No. It implements only a basic A profile (integer codec) also known as G.729a, and B profile, also known as G.729b.

B profile is essentially the same as A profile, but with CN (comfort noise), VAD (Voice Auto Detection), and DTX (Discontinuous Transmission) packets handling implemented. "A" profile decoder can handle "B" compliant stream by skipping unknown packets related to CN/VAD/DTX. "B" decoder can handle "A" compliant data w/o any issues.

A generic CN/VAD/DTX payload for codecs w/o support (for example - G.711 / G.722) is described in RFC 3389:

https://tools.ietf.org/html/rfc3389

Please also read G.729 wikipedia entry carefully. Especially non-technical part below the compatibility matrix and annex descriptions.

Just for the record - what people usually call G.729 is a different codec, although compatible, anyone can hardly find in the wild. Other G.729 extensions (C, D, etc) are also not so easy to find. If a codec can encode and decode G.729a stream this means that both parties can hear each other (unknown packets can be skipped, and any bigger "annex" codec can provide compatible output - fixed point, 8 kbit/s, no extensions).

G.729.1 is a totally different incompatible codec, and outside of the scope of this library.

(In reply to Tom "spot" Callaway from comment #22)
> Does bcg729 implement Annex C+, D, E, F, G, H, or I?

Tom, please take a look at the Patent's owner announcement:

http://www.sipro.com/G729.html

G.729 – IMPORTANT INFORMATION
As of January 1, 2017 the patent terms of most Licensed Patents under the G.729 Consortium have expired.

With regard to the unexpired Licensed Copyrights and Licensed Patents of the G.729 Consortium Patent License Agreement, the Licensors of the G.729 Consortium, namely Orange SA, Nippon Telegraph and Telephone Corporation and Université de Sherbrooke (“Licensors”) have agreed to license the same under the existing terms on a royalty-free basis starting January 1, 2017.

For current Licensees of the G.729 Consortium Patent License Agreement, no reports and no payments will be due for Licensed Products Sold or otherwise distributed as of January 1, 2017.
For other companies selling G.729 compliant products and that are not current Licensees of the G.729 Consortium, there is no need to execute a G.729 Consortium Patent License Agreement since Licensors have agreed to license the unexpired Licensed Copyrights and Licensed Patents of the G.729 Consortium Patent License Agreement under the existing terms on a royalty-free basis starting January 1, 2017.

I'd like to reopen this considering recent legal changes mentioned above. Just to make sure it will be on the radar.

Removing the FE-Legal block on this package. G.729 implementations are permissible in Fedora now.

Thanks! I'll go ahead and import the package then.

Mhh looks like I can't re-use this review (older than 60 days), need a re-review -> 1527953

*** This bug has been marked as a duplicate of bug 1527953 ***

(In reply to Tom "spot" Callaway from comment #26)
> Removing the FE-Legal block on this package. G.729 implementations are
> permissible in Fedora now.

Awesome news!