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

WebRTC: Solving the audio explosion issue in RTMP to RTC. #2011

Closed
wants to merge 1 commit into from

Conversation

PieerePi
Copy link
Contributor

@PieerePi PieerePi commented Nov 2, 2020

WebRTC: Solving the audio explosion issue in RTMP to RTC.


TRANS_BY_GPT3

Decoded audio frame is planar format, the linesize is larger than
the size of usable data. In old code, the second channel data is lost
and filled by noise.
Why copy frame sample by sample? :-(
@PieerePi PieerePi changed the title Fix webrtc audio noise. WebRTC:解决RTMP转RTC的爆音问题 Nov 2, 2020
@codecov-io
Copy link

codecov-io commented Nov 2, 2020

Codecov Report

Merging #2011 into 4.0release will increase coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           4.0release    #2011   +/-   ##
===========================================
  Coverage       57.17%   57.17%           
===========================================
  Files             125      125           
  Lines           52707    52707           
===========================================
+ Hits            30134    30135    +1     
+ Misses          22573    22572    -1     
Impacted Files Coverage Δ
trunk/src/app/srs_app_rtc_codec.cpp 0.00% <0.00%> (ø)
trunk/src/protocol/srs_service_utility.cpp 72.97% <0.00%> (+0.54%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
| Δ = absolute <relative> (impact), ø = not affected, ? = missing data |
Powered by Codecov. Last update a28f985...aca4f09. Read the comment docs.

TRANS_BY_GPT3

@winlinvip winlinvip changed the base branch from 4.0release to develop November 15, 2020 15:26
@wnpllrzodiac
Copy link
Contributor

old code only support mono audio?

@PieerePi
Copy link
Contributor Author

@wnpllrzodiac

No, the incoming audio is stereo.

It's a very common case, you can reproduce this issue easily. Use OBSStudio to publish stream to SRS, and use WebRTC to play.

(gdb) p *frame_
$7 = {data = {
0x2199b00 ""...,
0x219bb10 ""..., 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, linesize = {8192, 0, 0, 0, 0, 0, 0, 0}, extended_data = 0x20e4ec0, width = 0,
height = 0, nb_samples = 1024, format = 8, key_frame = 1, pict_type = AV_PICTURE_TYPE_NONE, sample_aspect_ratio = {num = 0, den = 1},

The size of first plane is 8192, but only has 1024 samples, that is 10244(FLTP) = 4096 bytes in total.
The size of second plane is 8192, but only has 1024 samples, that is 1024
4(FLTP) = 4096 bytes in total.

Since decoded audio frame is planar format, the linesize is greater than the size of useful data. In old code, the second channel data is lost and filled by another half useless data in first plane (that's noise).

@winlinvip Please have a look and merge this PR, thanks!

@winlinvip
Copy link
Member

winlinvip commented Jan 2, 2021

Thank you very much, we have already reviewed this PR and there is indeed this issue. I will look into merging it as soon as possible.

TRANS_BY_GPT3

@PieerePi
Copy link
Contributor Author

PieerePi commented Jan 3, 2021

Okay, thank you!
There is also a PR #2012, can you help take a look when you have time?

Also, you mentioned there is a discussion group for GB28181. How can I join it?

TRANS_BY_GPT3

@PieerePi PieerePi closed this Jan 25, 2021
@PieerePi PieerePi deleted the bugfix/webrtc_audio branch January 25, 2021 08:09
@winlinvip
Copy link
Member

See also #2172

@winlinvip
Copy link
Member

For #2106

@winlinvip winlinvip changed the title WebRTC:解决RTMP转RTC的爆音问题 WebRTC: Solving the audio explosion issue in RTMP to RTC. Jul 29, 2023
@winlinvip winlinvip added the TransByAI Translated by AI/GPT. label Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TransByAI Translated by AI/GPT.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants