Attachment 343325 Details for Bug 186307 – Patch

[patch] Patch

bug-186307-20180622082226.patch (text/plain), 8.62 KB, created by youenn fablet on 2018-06-22 08:22:26 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2018-06-22 08:22:26 PDT

Size: 8.62 KB

patch

obsolete

>Subversion Revision: 233079
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index b59540b0f63a819a0bb6c6ddf4ed862d28b9ac5d..e33e37ef6a2593ab6db3a8c792578d043b85f08d 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2018-06-22  Youenn Fablet  <youenn@apple.com>
>+
>+        Incoming G722 doesn't work
>+        https://bugs.webkit.org/show_bug.cgi?id=186307
>+        <rdar://problem/40809745>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        WebRTC backends usually does the following:
>+        - Initially call RealtimeIncomingAudioSource with 16KHz data
>+        - Switch to 48KHz when actual data is decoded.
>+        We added a check that was discarding any 16KHz data, but in case of G722, the data remains as 16KHz and is then never read.
>+        The solution is to remove the check that discards 16KHz information.
>+        We then need to fix a bug in AudioTrackPrivateMediaStreamCocoa that was preventing proper handling of change of audio data configuration.
>+
>+        Test: webrtc/audio-peer-connection-g722.html
>+
>+        * platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp:
>+        (WebCore::AudioTrackPrivateMediaStreamCocoa::audioSamplesAvailable):
>+        * platform/mediastream/mac/RealtimeIncomingAudioSourceCocoa.cpp:
>+        (WebCore::RealtimeIncomingAudioSourceCocoa::OnData):
>+
> 2018-06-20  Darin Adler  <darin@apple.com>
> 
>         [Cocoa] Use the isDirectory: variants of NSURL methods more to eliminate unnecessary file system activity
>diff --git a/Source/WebCore/platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp b/Source/WebCore/platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp
>index cbf88a462bb43dc608297d14658ebf1ef960daf1..87a65f26b1378a8f4714626739bf01f0a0394209 100644
>--- a/Source/WebCore/platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp
>+++ b/Source/WebCore/platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp
>@@ -194,14 +194,19 @@ void AudioTrackPrivateMediaStreamCocoa::audioSamplesAvailable(const MediaTime& s
>         m_inputDescription = std::make_unique<CAAudioStreamDescription>(inputDescription);
>         m_outputDescription = std::make_unique<CAAudioStreamDescription>(outputDescription);
> 
>-        if (!m_dataSource)
>-            m_dataSource = AudioSampleDataSource::create(description.sampleRate() * 2);
>+        m_dataSource = AudioSampleDataSource::create(description.sampleRate() * 2);
> 
>         if (m_dataSource->setInputFormat(inputDescription) || m_dataSource->setOutputFormat(outputDescription)) {
>             AudioComponentInstanceDispose(remoteIOUnit);
>             return;
>         }
> 
>+        if (m_isPlaying && AudioOutputUnitStart(remoteIOUnit)) {
>+            AudioComponentInstanceDispose(remoteIOUnit);
>+            m_inputDescription = nullptr;
>+            return;
>+        }
>+
>         m_dataSource->setVolume(m_volume);
>         m_remoteIOUnit = remoteIOUnit;
>     }
>diff --git a/Source/WebCore/platform/mediastream/mac/RealtimeIncomingAudioSourceCocoa.cpp b/Source/WebCore/platform/mediastream/mac/RealtimeIncomingAudioSourceCocoa.cpp
>index 119c9403dca1ea1de3a9190b3beb93307f192e89..22fb12561e8b9170f2f0ac9c47dfedcaaae5cb87 100644
>--- a/Source/WebCore/platform/mediastream/mac/RealtimeIncomingAudioSourceCocoa.cpp
>+++ b/Source/WebCore/platform/mediastream/mac/RealtimeIncomingAudioSourceCocoa.cpp
>@@ -68,15 +68,6 @@ static inline AudioStreamBasicDescription streamDescription(size_t sampleRate, s
> 
> void RealtimeIncomingAudioSourceCocoa::OnData(const void* audioData, int bitsPerSample, int sampleRate, size_t numberOfChannels, size_t numberOfFrames)
> {
>-    // We may receive OnData calls with empty sound data (mono, samples equal to zero and sampleRate equal to 16000) when starting the call.
>-    // FIXME: For the moment we skip them, we should find a better solution at libwebrtc level to not be called until getting some real data.
>-    if (sampleRate == 16000 && numberOfChannels == 1)
>-        return;
>-
>-    ASSERT(bitsPerSample == 16);
>-    ASSERT(numberOfChannels == 1 || numberOfChannels == 2);
>-    ASSERT(sampleRate == 48000);
>-
>     CMTime startTime = CMTimeMake(m_numberOfFrames, sampleRate);
>     auto mediaTime = PAL::toMediaTime(startTime);
>     m_numberOfFrames += numberOfFrames;
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index e37a4b213631544b4f0887c057420be5e3051e86..898fbe548da186568f9af4726854c81c96b53b4c 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-06-22  Youenn Fablet  <youenn@apple.com>
>+
>+        Incoming G722 doesn't work
>+        https://bugs.webkit.org/show_bug.cgi?id=186307
>+        <rdar://problem/40809745>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * webrtc/audio-peer-connection-g722-expected.txt: Added.
>+        * webrtc/audio-peer-connection-g722.html: Added.
>+        * webrtc/routines.js:
>+
> 2018-06-21  David Fenton  <david_fenton@apple.com>
> 
>         Skip imported/w3c/web-platform-tests/css/css-display/display-contents-first-letter-002.html.
>diff --git a/LayoutTests/webrtc/audio-peer-connection-g722-expected.txt b/LayoutTests/webrtc/audio-peer-connection-g722-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..f263245bf45bc8a5ea4f72dd1b4f0f282e1f9100
>--- /dev/null
>+++ b/LayoutTests/webrtc/audio-peer-connection-g722-expected.txt
>@@ -0,0 +1,3 @@
>+
>+PASS Basic G722 audio playback through a peer connection 
>+
>diff --git a/LayoutTests/webrtc/audio-peer-connection-g722.html b/LayoutTests/webrtc/audio-peer-connection-g722.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..78a6d80bb6865966cf3b4d881ec7f2ebafa07211
>--- /dev/null
>+++ b/LayoutTests/webrtc/audio-peer-connection-g722.html
>@@ -0,0 +1,68 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <meta charset="utf-8">
>+    <title>Testing local audio capture playback causes "playing" event to fire</title>
>+    <script src="../resources/testharness.js"></script>
>+    <script src="../resources/testharnessreport.js"></script>
>+    <script src ="routines.js"></script>
>+    <script>
>+var context = new webkitAudioContext();
>+var remoteStream;
>+
>+async function checkForHumBipBop(stream, previousResults, counter)
>+{
>+    if (!previousResults)
>+        previousResults = {
>+            heardHum : false,
>+            heardBip : false,
>+            heardBop : false
>+    };
>+    if (!counter)
>+        counter = 1;
>+    else if (++counter > 4)
>+        return Promise.resolve(false);
>+    results = await analyseAudio(stream, 1000, context);
>+    previousResults.heardHum |= results.heardHum;
>+    previousResults.heardBip |= results.heardBip;
>+    previousResults.heardBop |= results.heardBop;
>+    if (previousResults.heardHum && previousResults.heardBip && previousResults.heardBop)
>+        return Promise.resolve(true);
>+    var results = await checkForHumBipBop(stream, previousResults, counter);
>+    return results;
>+}
>+
>+function setCodec(sdp, codec)
>+{
>+    return sdp.split('\r\n').filter(line => {
>+        return line.indexOf('a=fmtp') === -1 && line.indexOf('a=rtcp-fb') === -1 && (line.indexOf('a=rtpmap') === -1 || line.indexOf(codec) !== -1);
>+    }).join('\r\n');
>+}
>+
>+promise_test(async (test) => {
>+    if (window.testRunner)
>+        testRunner.setUserMediaPermission(true);
>+
>+    const stream = await navigator.mediaDevices.getUserMedia({audio: true});
>+    const remoteStream = new Promise((resolve, reject) => {
>+        createConnections((firstConnection) => {
>+            firstConnection.addTrack(stream.getAudioTracks()[0], stream);
>+        }, (secondConnection) => {
>+            secondConnection.ontrack = (event) => { resolve(event.streams[0]); };
>+        }, { observeOffer : (offer) => {
>+            offer.sdp = setCodec(offer.sdp, "722");
>+            return offer;
>+        }
>+        });
>+        setTimeout(() => reject("Test timed out"), 5000);
>+    });
>+
>+    const results = await checkForHumBipBop(stream);
>+    assert_true(results, "heard hum bip bop");
>+    context.close();
>+}, "Basic G722 audio playback through a peer connection");
>+    </script>
>+</head>
>+<body>
>+</body>
>+</html>
>diff --git a/LayoutTests/webrtc/routines.js b/LayoutTests/webrtc/routines.js
>index 8d8cc6365be7a22b7c711d7f0ab425e2eda20519..aebe26fe67685b1cfe6bd637d72e99e6d74a9d5e 100644
>--- a/LayoutTests/webrtc/routines.js
>+++ b/LayoutTests/webrtc/routines.js
>@@ -31,8 +31,11 @@ function onCreateSessionDescriptionError(error)
> 
> function gotDescription1(desc, options)
> {
>-    if (options.observeOffer)
>-        options.observeOffer(desc);
>+    if (options.observeOffer) {
>+        const result = options.observeOffer(desc);
>+        if (result)
>+            desc = result;
>+    }
> 
>     localConnection.setLocalDescription(desc);
>     remoteConnection.setRemoteDescription(desc).then(() => {

Actions: View | Formatted Diff | Diff

Attachments on bug 186307: 343325 | 343330