Bug 218727 - Add support for AudioConfiguration.spatialRendering
Summary: Add support for AudioConfiguration.spatialRendering
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-11-09 15:16 PST by Jer Noble
Modified: 2020-11-10 11:10 PST (History)
10 users (show)

See Also:


Attachments
Patch (33.65 KB, patch)
2020-11-09 15:21 PST, Jer Noble
eric.carlson: review+
Details | Formatted Diff | Diff
Patch for landing (34.07 KB, patch)
2020-11-09 16:13 PST, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].