Bug 168118

Summary: [WebRTC] Implement Outgoing libwebrtc audio source support
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, jer.noble
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing none

Description youenn fablet 2017-02-10 08:31:20 PST
[WebRTC] Implement Outgoing LibWebRTCNetwork audio source support
Comment 1 youenn fablet 2017-02-10 09:29:15 PST
Created attachment 301161 [details]
Patch
Comment 2 youenn fablet 2017-02-10 10:14:15 PST
Created attachment 301168 [details]
Patch
Comment 3 youenn fablet 2017-02-10 10:52:21 PST
Created attachment 301176 [details]
Patch
Comment 4 youenn fablet 2017-02-10 10:55:12 PST
The story about muted/disabled tracks is not clear to me yet.
Should we continue being called by the capture thread?
Should we be notified and stop filling the ring buffer?
Or should we just not send any data to libwebrtc?
Comment 5 Eric Carlson 2017-02-10 12:37:49 PST
Comment on attachment 301176 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=301176&action=review

> Source/WebCore/ChangeLog:10
> +        Using an AudioSampleDataSource to convert the captured data in libwebrtc expected format.

Nit: "in" -> "to"

> Source/WebCore/ChangeLog:14
> +        Introducing bWebRTCAudioFormat.h to centrialise libwbebrtc expected audio format.

Nit: "bWebRTCAudioFormat.h" -> "WebRTCAudioFormat.h"

> Source/WebCore/platform/audio/mac/AudioSampleDataSource.cpp:289
> +bool AudioSampleDataSource::pullAvalaibleSamplesAsChunks(AudioBufferList& buffer, size_t sampleCountPerChunk, uint64_t timeStamp, Function<void()>&& bufferIsFilled)

Nit: "bufferIsFilled" reads like it should be a bool, something like "consumeSamples", or "processSamples" might be better.

> Source/WebCore/platform/mediastream/mac/RealtimeOutgoingAudioSource.h:70
> +    void audioSamplesAvailable(const MediaTime&,  const PlatformAudioData&, const AudioStreamDescription&, size_t) final;

Nit: extra space.
Comment 6 youenn fablet 2017-02-10 12:58:04 PST
Created attachment 301195 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2017-02-10 13:31:07 PST
Comment on attachment 301195 [details]
Patch for landing

Clearing flags on attachment: 301195

Committed r212144: <http://trac.webkit.org/changeset/212144>
Comment 8 WebKit Commit Bot 2017-02-10 13:31:11 PST
All reviewed patches have been landed.  Closing bug.