Bug 144538 – [WebAudio] Add experimental support for a streaming audio decoder.

• Status: NEW
• Component: New Bugs

[Jer Noble]

[WebAudio] Add experimental support for a streaming audio decoder.

[Jer Noble]

Created attachment 252257 [details]
Patch

[Build Bot]

Comment on attachment 252257 [details]
Patch

Attachment 252257 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5467842996600832

New failing tests:
js/dom/global-constructors-attributes.html

[Build Bot]

Created attachment 252258 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 252257 [details]
Patch

Attachment 252257 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6352887043063808

New failing tests:
js/dom/global-constructors-attributes.html
webaudio/audiostreamdecoder.html

[Build Bot]

Created attachment 252259 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Jer Noble]

Created attachment 252269 [details]
Patch

[Build Bot]

Comment on attachment 252269 [details]
Patch

Attachment 252269 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4505168490004480

New failing tests:
js/dom/global-constructors-attributes.html
webaudio/audiostreamdecoder.html

[Build Bot]

Created attachment 252271 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 252269 [details]
Patch

Attachment 252269 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4818787538829312

New failing tests:
js/dom/global-constructors-attributes.html

[Build Bot]

Created attachment 252272 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Jer Noble]

Created attachment 252273 [details]
Patch

[Jer Noble]

Created attachment 252274 [details]
Patch

[Build Bot]

Comment on attachment 252274 [details]
Patch

Attachment 252274 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6047085908459520

New failing tests:
js/dom/global-constructors-attributes.html
webaudio/audiostreamdecoder.html

[Build Bot]

Created attachment 252277 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 252274 [details]
Patch

Attachment 252274 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5132313172115456

New failing tests:
js/dom/global-constructors-attributes.html
webaudio/audiostreamdecoder.html

[Build Bot]

Created attachment 252280 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Jer Noble]

Created attachment 252288 [details]
Patch

[Jer Noble]

Finally ready for review (EWS are all green).

[Jer Noble]

Created attachment 270386 [details]
Patch

[Jer Noble]

Created attachment 270387 [details]
Patch

[Eric Carlson]

Comment on attachment 270387 [details]
Patch

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

> Source/WebCore/ChangeLog:1
> [phone]  Jer Noble  <[e-mail]>

This marinated for a while!

> Source/WebCore/Modules/webaudio/AudioStreamDecoder.cpp:2
> + * Copyright (C) 2015 Apple Inc. All rights reserved.

This should be updated.

> Source/WebCore/Modules/webaudio/AudioStreamDecoder.cpp:52
> +    // 1. If audioData is null or not a valid ArrayBuffer, return a rejected promise with a

I think it would be useful to include the url these steps came from to make it easier to review now and to check for spec changes in the future.

> Source/WebCore/Modules/webaudio/AudioStreamDecoder.h:2
> + * Copyright (C) 2015 Apple Inc. All rights reserved.

This should be updated.

> Source/WebCore/Modules/webaudio/AudioStreamDecoder.idl:2
> + * Copyright (C) 2015 Apple Inc. All rights reserved.

Ditto.

> Source/WebCore/bindings/js/JSAudioStreamDecoderCustom.cpp:2
> + * Copyright (C) 2015 Apple Inc. All rights reserved.

Ditto.

> Source/WebCore/platform/audio/AudioStreamReader.h:2
> + * Copyright (C) 2015 Apple Inc. All rights reserved.

Ditto.

> Source/WebCore/platform/audio/mac/AudioStreamReaderMac.h:2
> + * Copyright (C) 2015 Apple Inc. All rights reserved.

Ditto.

> Source/WebCore/platform/audio/mac/AudioStreamReaderMac.h:52
> +    dispatch_queue_t m_queue;

m_decodingQueue might be a better name to make its task clearer.

> Source/WebCore/platform/audio/mac/AudioStreamReaderMac.mm:2
> + * Copyright (C) 2015 Apple Inc. All rights reserved.

This should be updated.

> Source/WebCore/platform/audio/mac/AudioStreamReaderMac.mm:51
> +AudioStreamReaderMac::~AudioStreamReaderMac()
> +{
> +    if (m_stream) {
> +        OSStatus status = AudioFileStreamClose(m_stream);
> +        ASSERT(status == noErr);
> +        UNUSED_PARAM(status);
> +    }
> +}

The ChangeLog says "Close the dispatch queue." Is that not necessary?

[Jer Noble]

Comment on attachment 270387 [details]
Patch

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

>> Source/WebCore/Modules/webaudio/AudioStreamDecoder.cpp:2
>> + * Copyright (C) 2015 Apple Inc. All rights reserved.
> 
> This should be updated.

Okay, to this and all the other 2015 -> 2016 changes.

>> Source/WebCore/Modules/webaudio/AudioStreamDecoder.cpp:52
>> +    // 1. If audioData is null or not a valid ArrayBuffer, return a rejected promise with a
> 
> I think it would be useful to include the url these steps came from to make it easier to review now and to check for spec changes in the future.

Sure thing.

>> Source/WebCore/platform/audio/mac/AudioStreamReaderMac.h:52
>> +    dispatch_queue_t m_queue;
> 
> m_decodingQueue might be a better name to make its task clearer.

Ok.

>> Source/WebCore/platform/audio/mac/AudioStreamReaderMac.mm:51
>> +}
> 
> The ChangeLog says "Close the dispatch queue." Is that not necessary?

Whoops. At the time I wrote this, I didn't know about OSObjectPtr (a RetainPtr<> for things like dispatch_queue_t). I'll add that here so that the dispatch queue doesn't need to be explicitly released.

[Jer Noble]

Created attachment 270391 [details]
Patch for landing

[Jer Noble]

Created attachment 270496 [details]
Patch for landing

[Build Bot]

Comment on attachment 270496 [details]
Patch for landing

Attachment 270496 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/773529

New failing tests:
js/dom/global-constructors-attributes.html
webaudio/audiostreamdecoder.html

[Build Bot]

Created attachment 270501 [details]
Archive of layout-test-results from ews100 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 270496 [details]
Patch for landing

Attachment 270496 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/773532

New failing tests:
js/dom/global-constructors-attributes.html
webaudio/audiostreamdecoder.html

[Build Bot]

Created attachment 270502 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 270496 [details]
Patch for landing

Attachment 270496 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/773528

New failing tests:
js/dom/global-constructors-attributes.html
webaudio/audiostreamdecoder.html

[Build Bot]

Created attachment 270503 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Radar WebKit Bug Importer]

<rdar://problem/25513370>