-
Notifications
You must be signed in to change notification settings - Fork 389
Fix timestamps between contiguous mixed audio packets may vary #199
Conversation
| frame.timeStamp = (AudioTime::currentTime()) * m_rtpSampleRate / 1000; | ||
|
|
||
| int64_t computedTimestamp = AudioTime::currentTime() * m_rtpSampleRate / 1000; | ||
| int64_t sizeFor10Ms = m_rtpSampleRate * 10 / 1000 * frame.additionalInfo.audio.channels; //size for 10 ms, such as 48000 * 10/1000 * 2 = 960 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
multiply by frame.additionalInfo.audio.channels is not correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx, would you please tell me why it's not correct?
daijh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The interval of OPUS frame is 20ms, not 10ms.
Please try to use timestamp in SendData() callback.
|
@daijh Thanks for your review. Maybe I misunderstood the code. I tried to print timestamp parameter in SendData() function, it looks like this, the diff between two contiguous timestamps is 2880, I don't know what does it meaning and how I should use them: (In our case, we use samplerate 48000) |
|
Could you provide more info so I can reproduce on my side and try to improve it? |
|
In our case, to reproduce this problem is very simple , and we can reproduce it by 100%, it only need 2 mobile clients, the first one publish a audio stream which plays a music, and if the second one subscribe the first one's single stream, the music sounds good, but if the second one subscribe the mixed stream, the music sounds very bad, the beats is a mass. My fix can resolve this problem, its key goal is to keep the diff of two timestamp constantly, but if it's not correct for all cases, would you please improve it for us and other users? |
|
Extra info: |
Hi @daijh, is there any progresses? |
|
We prepared two patches to improve the timestamp, still under review and testing. |
Thanks very much! |
|
@qiaoxueshi We are about to close this PR since @daijh has fixed this issue with PR #213(#213) on this repo and PR #14(open-webrtc-toolkit/owt-deps-webrtc#14) on owt-deps-webrtc. |
Thanks! |
We found timestamps between contiguous mixed audio packets may vary, which will mislead the client WebRTC neteq module make too much accelerations and expansions, and lead to the rhythm of music is very unstable, so I do this fix to make the timestamps between contiguous mixed audio packets stable as much as possible.