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.

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

[Feature] Add AAC Support #1966

Open
wants to merge 11 commits into
base: develop-pre-1.11.0
Choose a base branch
from
Open

[Feature] Add AAC Support #1966

wants to merge 11 commits into from

Conversation

niyatim23
Copy link
Contributor

@niyatim23 niyatim23 commented Apr 7, 2024

Issue #, if available:

What was changed?

  • Added support for AAC in RTP and SDP

Why was it changed?

  • This is new feature to expand support for different codecs

How was it changed?

  • Introduced AAC payloader and de-payloader in RTP
  • Modified the SessionDescription.c to include AAC in offer and answer

What testing was done for the changes?

build % ffprobe -v error -show_entries stream=codec_name,sample_rate,width,height,channels -of default=noprint_wrappers=1 video.mkv 
codec_name=h264
width=1280
height=720
codec_name=aac
sample_rate=16000
channels=2

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@niyatim23 niyatim23 added enhancement New feature or request work-in-progress labels Apr 7, 2024
@niyatim23 niyatim23 marked this pull request as ready for review April 16, 2024 17:45
@codecov-commenter
Copy link

codecov-commenter commented May 7, 2024

Codecov Report

Attention: Patch coverage is 64.91228% with 20 lines in your changes are missing coverage. Please review.

Project coverage is 88.51%. Comparing base (a28e1e4) to head (8f5b0c1).

❗ Current head 8f5b0c1 differs from pull request most recent head 2f21e35. Consider uploading reports for the commit 2f21e35 to get more accurate results

Files Patch % Lines
src/source/PeerConnection/PeerConnection.c 0.00% 6 Missing ⚠️
src/source/PeerConnection/SessionDescription.c 33.33% 6 Missing ⚠️
src/source/PeerConnection/Rtp.c 0.00% 4 Missing ⚠️
src/source/Rtp/Codecs/RtpAacPayloader.c 89.47% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1966      +/-   ##
===========================================
- Coverage    88.60%   88.51%   -0.09%     
===========================================
  Files           47       48       +1     
  Lines        12697    12750      +53     
===========================================
+ Hits         11250    11286      +36     
- Misses        1447     1464      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -44,6 +45,7 @@ extern "C" {
#define DEFAULT_PAYLOAD_MULAW (UINT64) 0
#define DEFAULT_PAYLOAD_ALAW (UINT64) 8
#define DEFAULT_PAYLOAD_OPUS (UINT64) 111
#define DEFAULT_PAYLOAD_AAC (UINT64) 96
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably remove these hardcoded payload types altogether: https://issues.webrtc.org/issues/42231779
Not necessary for part of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're using it as a part of setPayloadTypesForOffer actually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants