MediaRecorder should allow setting low bit rates for audio

https://bugs.webkit.org/show_bug.cgi?id=216688 <rdar://problem/69129142> Reviewed by Eric Carlson. Source/WebCore: When setting the bitrate provided by the web app fails, we now use default bit rate values that are expected to work properly. Covered by updated tests. * platform/mediarecorder/cocoa/AudioSampleBufferCompressor.h: * platform/mediarecorder/cocoa/AudioSampleBufferCompressor.mm: (WebCore::AudioSampleBufferCompressor::setBitsPerSecond): (WebCore::AudioSampleBufferCompressor::defaultOutputBitRate const): (WebCore::AudioSampleBufferCompressor::initAudioConverterForSourceFormatDescription): LayoutTests: Add more bitrate tests. * http/wpt/mediarecorder/MediaRecorder-audio-bitrate.html: Canonical link: https://commits.webkit.org/229941@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@267822 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Oct 1, 2020 · 8df57f1 · 8df57f1
1 parent 2d6174a
commit 8df57f1
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 23 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2020-10-01  Youenn Fablet  <youenn@apple.com>
+
+        MediaRecorder should allow setting low bit rates for audio
+        https://bugs.webkit.org/show_bug.cgi?id=216688
+        <rdar://problem/69129142>
+
+        Reviewed by Eric Carlson.
+
+        Add more bitrate tests.
+
+        * http/wpt/mediarecorder/MediaRecorder-audio-bitrate.html:
+
 2020-09-30  Lauro Moura  <lmoura@igalia.com>
 
         [GLIB] Missing bits from last gardening

diff --git a/LayoutTests/http/wpt/mediarecorder/MediaRecorder-audio-bitrate.html b/LayoutTests/http/wpt/mediarecorder/MediaRecorder-audio-bitrate.html
@@ -21,18 +21,22 @@
 
     promise_test(async (t) => {
         const stream = await navigator.mediaDevices.getUserMedia({ audio : true });
-        const bitRates = [128000, 192000, 256000];
+        const bitRates = [128000, 192000, 256000, 12000, 24000, 108555];
         let promises = [];
         bitRates.forEach(bitRate => {
             promises.push(record(stream, bitRate));
         });
 
-        let blobs = [0, 0, 0];
+        let blobs = new Array(bitRates.length).fill(0);
         promises.forEach(async (promise, index) => {
             blobs[index] = await promise;
         });
         await Promise.all(promises);
-        assert_not_equals(blobs[0].size, 0);
+
+        let counter = 0;
+        blobs.forEach(blob => {
+            assert_not_equals(blob.size, 0, "bitrate " + bitRates[counter++]);
+        });
         assert_greater_than(blobs[2].size, blobs[0].size);
     }, "Various audio bitrates");
 </script>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,20 @@
+2020-10-01  Youenn Fablet  <youenn@apple.com>
+
+        MediaRecorder should allow setting low bit rates for audio
+        https://bugs.webkit.org/show_bug.cgi?id=216688
+        <rdar://problem/69129142>
+
+        Reviewed by Eric Carlson.
+
+        When setting the bitrate provided by the web app fails, we now use default bit rate values that are expected to work properly.
+        Covered by updated tests.
+
+        * platform/mediarecorder/cocoa/AudioSampleBufferCompressor.h:
+        * platform/mediarecorder/cocoa/AudioSampleBufferCompressor.mm:
+        (WebCore::AudioSampleBufferCompressor::setBitsPerSecond):
+        (WebCore::AudioSampleBufferCompressor::defaultOutputBitRate const):
+        (WebCore::AudioSampleBufferCompressor::initAudioConverterForSourceFormatDescription):
+
 2020-09-30  Rob Buis  <rbuis@igalia.com>
 
         Remove responseIsXML

diff --git a/Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.h b/Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.h
@@ -48,7 +48,7 @@ class AudioSampleBufferCompressor {
 private:
     AudioSampleBufferCompressor();
     bool initialize(CMBufferQueueTriggerCallback, void* callbackObject);
-    UInt32 outputBitRate(const AudioStreamBasicDescription&) const;
+    UInt32 defaultOutputBitRate(const AudioStreamBasicDescription&) const;
 
     static OSStatus audioConverterComplexInputDataProc(AudioConverterRef, UInt32*, AudioBufferList*, AudioStreamPacketDescription**, void*);
 

diff --git a/Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.mm b/Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.mm
@@ -99,25 +99,11 @@
 
 void AudioSampleBufferCompressor::setBitsPerSecond(unsigned bitRate)
 {
-    // FIXME: we have some issues when setting up some bit rates, only allow some that work for the moment.
-    if (bitRate < 128000) {
-        RELEASE_LOG_INFO(WebRTC, "AudioSampleBufferCompressor::outputBitRate clamped to 128000.");
-        bitRate = 128000;
-    } else if (bitRate > 256000) {
-        RELEASE_LOG_INFO(WebRTC, "AudioSampleBufferCompressor::outputBitRate clamped to 256000.");
-        bitRate = 256000;
-    } else if (bitRate != 128000 && bitRate != 192000 && bitRate != 256000) {
-        RELEASE_LOG_INFO(WebRTC, "AudioSampleBufferCompressor::outputBitRate did not set output bit rate as value is not supported.");
-        return;
-    }
     m_outputBitRate = bitRate;
 }
 
-UInt32 AudioSampleBufferCompressor::outputBitRate(const AudioStreamBasicDescription& destinationFormat) const
+UInt32 AudioSampleBufferCompressor::defaultOutputBitRate(const AudioStreamBasicDescription& destinationFormat) const
 {
-    if (m_outputBitRate)
-        return *m_outputBitRate;
-
     if (destinationFormat.mSampleRate >= 44100)
         return 192000;
     if (destinationFormat.mSampleRate < 22000)
@@ -180,10 +166,18 @@
     }
 
     if (m_destinationFormat.mFormatID == kAudioFormatMPEG4AAC) {
-        auto outputBitRate = this->outputBitRate(m_destinationFormat);
-        size = sizeof(outputBitRate);
-        if (auto error = AudioConverterSetProperty(m_converter, kAudioConverterEncodeBitRate, size, &outputBitRate))
-            RELEASE_LOG_ERROR(MediaStream, "AudioSampleBufferCompressor setting kAudioConverterEncodeBitRate failed with %d", error);
+        bool shouldSetDefaultOutputBitRate = true;
+        if (m_outputBitRate) {
+            auto error = AudioConverterSetProperty(m_converter, kAudioConverterEncodeBitRate, sizeof(*m_outputBitRate), &m_outputBitRate.value());
+            RELEASE_LOG_ERROR_IF(error, MediaStream, "AudioSampleBufferCompressor setting kAudioConverterEncodeBitRate failed with %d", error);
+            shouldSetDefaultOutputBitRate = !!error;
+        }
+        if (shouldSetDefaultOutputBitRate) {
+            auto outputBitRate = defaultOutputBitRate(m_destinationFormat);
+            size = sizeof(outputBitRate);
+            if (auto error = AudioConverterSetProperty(m_converter, kAudioConverterEncodeBitRate, size, &outputBitRate))
+                RELEASE_LOG_ERROR(MediaStream, "AudioSampleBufferCompressor setting default kAudioConverterEncodeBitRate failed with %d", error);
+        }
     }
 
     if (!m_destinationFormat.mBytesPerPacket) {