Bug 134877 – [Mac] don't enable low power audio mode on external output devices

• Component: Media
• Status: RESOLVED FIXED

[Eric Carlson]

Firewire and other "high end" output devices behave poorly when asked to use a 4K output buffer.

<rdar://17443302>

[Eric Carlson]

Created attachment 234839 [details]
Proposed patch

[WebKit Commit Bot]

Attachment 234839 [details] did not pass style-queue:


Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jer Noble]

I think you may have just uploaded the ChangeLog.  Like, all of it. :)

[Build Bot]

Comment on attachment 234839 [details]
Proposed patch

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

New failing tests:
media/W3C/video/networkState/networkState_during_loadstart.html

[Build Bot]

Created attachment 234842 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

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

[Eric Carlson]

Created attachment 234862 [details]
This time with an actual patch

[Jer Noble]

Comment on attachment 234862 [details]
This time with an actual patch

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

> Source/WebCore/platform/audio/mac/MediaSessionManagerMac.cpp:60
> +
> +        size_t bufferSize;
> +        if (m_audioHardwareListener && m_audioHardwareListener->outputDeviceSupportsLowPowerMode())
> +            bufferSize = kLowPowerVideoBufferSize;
> +        else
> +            bufferSize = kWebAudioBufferSize;
> +
> +        AudioSession::sharedSession().setPreferredBufferSize(bufferSize);

I think this deserves a follow-up.  The kWebAudioBufferSize value will be lower than the system default. That should be fine, as far as this bug is concerned, but we should add something later which retrieves the default size, or removes the property we set.

[Eric Carlson]

(In reply to comment #7)
> (From update of attachment 234862 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=234862&action=review
> 
> > Source/WebCore/platform/audio/mac/MediaSessionManagerMac.cpp:60
> > +
> > +        size_t bufferSize;
> > +        if (m_audioHardwareListener && m_audioHardwareListener->outputDeviceSupportsLowPowerMode())
> > +            bufferSize = kLowPowerVideoBufferSize;
> > +        else
> > +            bufferSize = kWebAudioBufferSize;
> > +
> > +        AudioSession::sharedSession().setPreferredBufferSize(bufferSize);
> 
> I think this deserves a follow-up.  The kWebAudioBufferSize value will be lower than the system default. That should be fine, as far as this bug is concerned, but we should add something later which retrieves the default size, or removes the property we set.

Good point, I will file a bug to fix this in a follow-up.

[WebKit Commit Bot]

Comment on attachment 234862 [details]
This time with an actual patch

Rejecting attachment 234862 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 234862, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebKit2/ChangeLog contains OOPS!.

Full output: http://webkit-queues.appspot.com/results/6070414644084736

[Eric Carlson]

Committed r171069: : https://trac.webkit.org/r171069

[Csaba Osztrogonác]

(In reply to comment #10)
> Committed r171069: : https://trac.webkit.org/r171069

FYI: It broke the Apple Windows build.

[Brent Fulgham]

Build fix committed:

Committed r171116: : https://trac.webkit.org/r171116