Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for the Opus codec #179

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add support for the Opus codec #179

wants to merge 7 commits into from

Conversation

fbriere
Copy link
Collaborator

@fbriere fbriere commented Dec 22, 2019

Closes #101

@fbriere fbriere added enhancement untested This pull resquest has not (completely) been tested codec labels Dec 22, 2019
@fbriere
Copy link
Collaborator Author

fbriere commented Dec 22, 2019

A few additional details:

  • Forward Error Correction (FEC) is not supported on the decoding side, and any such data in the incoming stream will simply be discarded. This is unlikely to change, given Twinkle's design. (It will still be enabled on the encoding side if the other party requests it, though.)

  • Opus only allows a few specific ptime values; the configured value will be rounded up if necessary (to a maximum of 60 ms).

  • Any maxptime attribute received during SDP will be ignored (possibly in violation of RFC 7587, s. 7.1), since Twinkle was never updated to take RFC 4566 into account.

  • Despite Opus being a "successor" to Speex, it is only used as a codec proper, and any pre-processing is still handled by the latter.

@fbriere fbriere mentioned this pull request Dec 22, 2019
Open
@christf
Copy link

christf commented Apr 21, 2021

what can be done to bring this forward?

@petrov-adg
Copy link

@fbriere Great work ! Opus work's very well. Can you please merge this changes into master ?

@fbriere fbriere force-pushed the feature/opus branch 2 times, most recently from a599bc5 to 8626a03 Compare February 13, 2022 01:57
@fbriere
Copy link
Collaborator Author

fbriere commented Feb 16, 2022

@petrov-adg Thanks for your positive feedback! I'll go ahead and remove the untested label.

@fbriere fbriere removed the untested This pull resquest has not (completely) been tested label Feb 16, 2022
@fbriere fbriere mentioned this pull request Feb 23, 2022
Open
@fbriere
Extract the (optional) number of channels from the rtpmap codec name
ed6a75b
@fbriere
Reconstruct the parameters string in t_sdp::get_fmtp()
a108d74 
t_sdp::get_fmtp() previously assumed that any "a=fmtp:" line contained
only two tokens, but RFC 4566 does not preclude the presence of spaces
in the "format specific parameters" part.  In fact, there are many
examples (such as found in RFC 6871) where multiple parameters are
separated by "; ", thus creating several tokens.

The proper fix would probably be to introduce a `maxsplit` argument to
split_ws() in order to keep the whole string intact, but that would be a
bit intrusive.  Simply putting some spaces back to reconstruct the
string to a somewhat equivalent version will be enough for our needs.
@fbriere
Add t_sdp::set_fmtp_int_params(), to add several fmtp params at once
4289d91
@fbriere
Make SDP format parameters available to any encoder/decoder
5515b8c
@fbriere
Add support for the Opus codec
b00f847 
Closes LubosD#101
@fbriere
Don't use IMPORTED_TARGET for pkg_check_modules() in CMakeLists.txt
60b9274 
`IMPORTED_TARGET` was only introduced in CMake 3.6.
@fbriere
Remove the name of all <spacer> and <layout> elements
968db57 
These generic names added by Qt Designer do nothing but cause conflicts.
@fbriere
Copy link
Collaborator Author

fbriere commented Jun 9, 2022

This branch is now rebased and ready for merging.

(The conflict was merely due to an already-applied commit. I'm a bit surprised that GitHub would be stumped by this.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
codec enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Opus codec
3 participants
@fbriere @christf @petrov-adg