Bug 218727

Summary: Add support for AudioConfiguration.spatialRendering
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, eric.carlson, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, peng.liu6, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
eric.carlson: review+
Patch for landing none

Description Jer Noble 2020-11-09 15:16:44 PST
Add support for AudioConfiguration.spatialRendering
Comment 1 Radar WebKit Bug Importer 2020-11-09 15:17:24 PST
<rdar://problem/71213348>
Comment 2 Jer Noble 2020-11-09 15:21:41 PST
Created attachment 413636 [details]
Patch
Comment 3 Eric Carlson 2020-11-09 16:06:10 PST
Comment on attachment 413636 [details]
Patch

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

> Source/WebCore/PAL/pal/avfoundation/OutputContext.mm:42
> +    : m_context(context)

WTFMove(context)

> Source/WebCore/PAL/pal/avfoundation/OutputContext.mm:90
> +        if (auto* outputDevice = [m_context outputDevice])

Nit: '*' on the wrong side.

> Source/WebCore/PAL/pal/avfoundation/OutputContext.mm:95
> +    auto* avOutputDevices = [m_context outputDevices];

Ditto

> Source/WebCore/PAL/pal/avfoundation/OutputDevice.mm:47
> +uint8_t OutputDevice::deviceFeatures() const

You could use std::bitset<>
Comment 4 Peng Liu 2020-11-09 16:09:40 PST
Comment on attachment 413636 [details]
Patch

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

> Source/WebCore/PAL/pal/avfoundation/OutputContext.h:38
> +class OutputContext {

Is this for audio output only?

> Source/WebCore/PAL/pal/avfoundation/OutputContext.mm:48
> +    static NeverDestroyed<Optional<OutputContext>> sharedAudioPresentationOutputContext = [] () -> Optional<OutputContext> {

Nit. The space between [] and () can be removed.

> Source/WebCore/PAL/pal/avfoundation/OutputContext.mm:103
> +

Nit. An extra space.

> Source/WebCore/platform/graphics/cocoa/MediaEngineConfigurationFactoryCocoa.cpp:133
> +            if (!context || !WTF::allOf(context->outputDevices(), [] (auto& device) {

Nit. The space between [] and () can be removed.
Comment 5 Jer Noble 2020-11-09 16:13:40 PST
Created attachment 413638 [details]
Patch for landing
Comment 6 Jer Noble 2020-11-09 16:15:10 PST
(In reply to Peng Liu from comment #4)
> Comment on attachment 413636 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=413636&action=review
> 
> > Source/WebCore/PAL/pal/avfoundation/OutputContext.h:38
> > +class OutputContext {
> 
> Is this for audio output only?

Not necessarily; it is also used for remote video playback over AirPlay or through an external TV screen.

> > Source/WebCore/PAL/pal/avfoundation/OutputContext.mm:48
> > +    static NeverDestroyed<Optional<OutputContext>> sharedAudioPresentationOutputContext = [] () -> Optional<OutputContext> {
> 
> Nit. The space between [] and () can be removed.

Won't the style checker complain about that?
Comment 7 EWS 2020-11-10 11:10:55 PST
Committed r269631: <https://trac.webkit.org/changeset/269631>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 413638 [details].