Always provide as much audio data from PeerConnection as MediaStreamGraph asks for

RESOLVED FIXED in mozilla20

Status

()

Core
WebRTC: Audio/Video
P2
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mreavy, Assigned: ekr)

Tracking

unspecified
mozilla20
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [WebRTC], [blocking-webrtc+] [qa-])

Attachments

(2 attachments)

The GIPS code always provides audio data in 10ms fixed buffers.  MediaStreamGraph may ask for audio in any buffer size.  Even if MediaStreamGraph requests just slightly more than 10ms, provide the full second 10ms buffer and let the rest of the code handle it.  The jitter buffer will grow, but that's ok.  We can worry about optimizing the sizes requested by MediaStreamGraph in a separate bug.

Implementing this fix should eliminate some (hopefully all) of the audio artifacts we're hearing from the output side of the current PeerConnection code.
See bug 822956 comment 23 and following
Assignee: nobody → ekr
(Assignee)

Comment 2

5 years ago
Created attachment 698200 [details] [diff] [review]
Have MediaPipeline deliver as much media as requested
(Assignee)

Updated

5 years ago
Attachment #698200 - Flags: review?(rjesup)
Comment on attachment 698200 [details] [diff] [review]
Have MediaPipeline deliver as much media as requested

Review of attachment 698200 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
@@ +828,5 @@
>      MOZ_MTLOG(PR_LOG_ERROR, "NotifyPull() called from a non-SourceMediaStream");
>      return;
>    }
>  
> +  while (MillisecondsToMediaTime(played_) < desired_time) {

Just add a comment that that it's done this way to avoid roundoff errors from accumulating.
Attachment #698200 - Flags: review?(rjesup) → review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9958b17acc01
(Assignee)

Comment 5

5 years ago
Created attachment 698322 [details] [diff] [review]
Fix unit tests to match fix to MediaPipeline
(Assignee)

Comment 6

5 years ago
Comment on attachment 698322 [details] [diff] [review]
Fix unit tests to match fix to MediaPipeline

Forget to change the unit tests. Also a bit of trivial reformatting
Attachment #698322 - Flags: review?(rjesup)

Updated

5 years ago
Attachment #698322 - Flags: review?(rjesup) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b88be2c5e1f
https://hg.mozilla.org/mozilla-central/rev/9958b17acc01
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20

Updated

5 years ago
Whiteboard: [WebRTC], [blocking-webrtc+] → [WebRTC], [blocking-webrtc+] [qa-]

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/1b88be2c5e1f
You need to log in before you can comment on or make changes to this bug.