Skip to content

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

Copy link

This is a very old issue. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to open a new one.

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