WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
116342
Mac: Set the default audio buffer size to a large value for <video> elements.
https://bugs.webkit.org/show_bug.cgi?id=116342
Summary
Mac: Set the default audio buffer size to a large value for <video> elements.
Jer Noble
Reported
2013-05-17 12:39:39 PDT
Mac: Set the default audio buffer size to a large value for <video> elements.
Attachments
Patch
(35.62 KB, patch)
2013-05-17 13:11 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(36.82 KB, patch)
2013-05-17 14:48 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(41.97 KB, patch)
2013-05-19 22:39 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(41.93 KB, patch)
2013-05-19 22:58 PDT
,
Jer Noble
eric.carlson
: review+
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Jer Noble
Comment 1
2013-05-17 13:11:49 PDT
Created
attachment 202141
[details]
Patch
Build Bot
Comment 2
2013-05-17 13:39:33 PDT
Comment on
attachment 202141
[details]
Patch
Attachment 202141
[details]
did not pass win-ews (win): Output:
http://webkit-queues.appspot.com/results/484804
EFL EWS Bot
Comment 3
2013-05-17 13:40:30 PDT
Comment on
attachment 202141
[details]
Patch
Attachment 202141
[details]
did not pass efl-wk2-ews (efl-wk2): Output:
http://webkit-queues.appspot.com/results/487539
EFL EWS Bot
Comment 4
2013-05-17 13:41:07 PDT
Comment on
attachment 202141
[details]
Patch
Attachment 202141
[details]
did not pass efl-ews (efl): Output:
http://webkit-queues.appspot.com/results/492451
Build Bot
Comment 5
2013-05-17 13:44:29 PDT
Comment on
attachment 202141
[details]
Patch
Attachment 202141
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/484800
Early Warning System Bot
Comment 6
2013-05-17 14:19:24 PDT
Comment on
attachment 202141
[details]
Patch
Attachment 202141
[details]
did not pass qt-ews (qt): Output:
http://webkit-queues.appspot.com/results/486563
Early Warning System Bot
Comment 7
2013-05-17 14:25:19 PDT
Comment on
attachment 202141
[details]
Patch
Attachment 202141
[details]
did not pass qt-wk2-ews (qt-wk2): Output:
http://webkit-queues.appspot.com/results/492464
Build Bot
Comment 8
2013-05-17 14:39:17 PDT
Comment on
attachment 202141
[details]
Patch
Attachment 202141
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/480981
Jer Noble
Comment 9
2013-05-17 14:48:52 PDT
Created
attachment 202160
[details]
Patch Added new files to the various build files for other platforms.
Early Warning System Bot
Comment 10
2013-05-17 14:53:40 PDT
Comment on
attachment 202160
[details]
Patch
Attachment 202160
[details]
did not pass qt-ews (qt): Output:
http://webkit-queues.appspot.com/results/483900
Early Warning System Bot
Comment 11
2013-05-17 14:55:45 PDT
Comment on
attachment 202160
[details]
Patch
Attachment 202160
[details]
did not pass qt-wk2-ews (qt-wk2): Output:
http://webkit-queues.appspot.com/results/482925
EFL EWS Bot
Comment 12
2013-05-17 15:07:46 PDT
Comment on
attachment 202160
[details]
Patch
Attachment 202160
[details]
did not pass efl-ews (efl): Output:
http://webkit-queues.appspot.com/results/493427
Build Bot
Comment 13
2013-05-17 15:13:58 PDT
Comment on
attachment 202160
[details]
Patch
Attachment 202160
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/497144
Build Bot
Comment 14
2013-05-17 15:25:02 PDT
Comment on
attachment 202160
[details]
Patch
Attachment 202160
[details]
did not pass win-ews (win): Output:
http://webkit-queues.appspot.com/results/494426
EFL EWS Bot
Comment 15
2013-05-17 15:35:26 PDT
Comment on
attachment 202160
[details]
Patch
Attachment 202160
[details]
did not pass efl-wk2-ews (efl-wk2): Output:
http://webkit-queues.appspot.com/results/482939
Build Bot
Comment 16
2013-05-17 16:23:03 PDT
Comment on
attachment 202160
[details]
Patch
Attachment 202160
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/494442
kov's GTK+ EWS bot
Comment 17
2013-05-18 02:00:35 PDT
Comment on
attachment 202160
[details]
Patch
Attachment 202160
[details]
did not pass gtk-ews (gtk): Output:
http://webkit-queues.appspot.com/results/499155
Jer Noble
Comment 18
2013-05-19 22:39:17 PDT
Created
attachment 202258
[details]
Patch Instead of adding what is effectively a no-op class to every port, protect non-implementing ports by wrapping AudioSession and its related classes in USE(AUDIO_SESSION) checks.
Jer Noble
Comment 19
2013-05-19 22:58:01 PDT
Created
attachment 202259
[details]
Patch Fix windows build failure.
Eric Carlson
Comment 20
2013-05-22 11:12:52 PDT
Comment on
attachment 202259
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=202259&action=review
> Source/WebCore/ChangeLog:9 > + requested buffer frame size to a large value, such as 4096. Since this results
"such as 4096" - kPreferredLowPowerBufferSize is set to 4098?
> Source/WebCore/platform/audio/AudioSessionManager.cpp:61 > + ASSERT(type >= 0);
Is "type >= 0" really useful? Asserting that it is one of the valid values would be more complete - "ASSERT(type >= None && type <= WebAudio)"
> Source/WebCore/platform/audio/AudioSessionManager.cpp:67 > + ASSERT(type >= 0);
Ditto.
> Source/WebCore/platform/audio/AudioSessionManager.cpp:74 > + ASSERT(type >= 0);
Ditto.
> Source/WebCore/platform/audio/ios/AudioDestinationIOS.cpp:47 > +#if USE(AUDIO_SESSION)
Is it possible that we would ever want to build this without "AUDIO_SESSION"? If not, remove the #ifs and let the build fail if someone tries.
> Source/WebCore/platform/audio/ios/AudioSessionIOS.mm:29 > +#if USE(AUDIO_SESSION) && PLATFORM(IOS)
Ditto.
> Source/WebCore/platform/audio/mac/AudioDestinationMac.cpp:41 > +#if USE(AUDIO_SESSION)
Ditto.
> Source/WebCore/platform/audio/mac/AudioDestinationMac.cpp:74 > +#if USE(AUDIO_SESSION) > + return AudioSession::sharedSession().sampleRate(); > +#else > + return 0; > +#endif
OTOH, if it is possible that someone would want to build this without AUDIO_SESSION shouldn't this include the old code in the #else?
Jer Noble
Comment 21
2013-05-22 11:33:26 PDT
Comment on
attachment 202259
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=202259&action=review
>> Source/WebCore/ChangeLog:9 >> + requested buffer frame size to a large value, such as 4096. Since this results > > "such as 4096" - kPreferredLowPowerBufferSize is set to 4098?
Whoops, it should be 4096.
>> Source/WebCore/platform/audio/AudioSessionManager.cpp:61 >> + ASSERT(type >= 0); > > Is "type >= 0" really useful? Asserting that it is one of the valid values would be more complete - "ASSERT(type >= None && type <= WebAudio)"
This is really asserting that no one adds a type to the AudioType enum that is less than zero. Because the HashMap can't handle value types that are unknown enums, i used an unsigned value (size_t), and want to guarantee no one adds an incompatible enum value. I guess it doesn't really matter, since it will probably just cast a negative enum to a wrapped-around unsigned number. I'll remove it.
>> Source/WebCore/platform/audio/ios/AudioDestinationIOS.cpp:47 >> +#if USE(AUDIO_SESSION) > > Is it possible that we would ever want to build this without "AUDIO_SESSION"? If not, remove the #ifs and let the build fail if someone tries.
No, we will never want to use this without AUDIO_SESSION. I'll remove all these #ifs.
>> Source/WebCore/platform/audio/mac/AudioDestinationMac.cpp:74 >> +#endif > > OTOH, if it is possible that someone would want to build this without AUDIO_SESSION shouldn't this include the old code in the #else?
Well, someone shouldn't do that, right? :)
Jer Noble
Comment 22
2013-05-22 16:43:13 PDT
Committed
r150555
: <
http://trac.webkit.org/changeset/150555
>
Ryosuke Niwa
Comment 23
2013-05-22 18:16:40 PDT
(In reply to
comment #22
)
> Committed
r150555
: <
http://trac.webkit.org/changeset/150555
>
This patch might have broken 30 media tests:
http://build.webkit.org/builders/Apple%20Lion%20Release%20WK1%20%28Tests%29/builds/12699
Benjamin Poulain
Comment 24
2013-05-22 18:18:58 PDT
Comment on
attachment 202259
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=202259&action=review
> Source/WebCore/html/HTMLMediaElement.cpp:190 > +static const size_t kPreferredLowPowerBufferSize = 4098;
Where is this used?
Ryosuke Niwa
Comment 25
2013-05-22 18:34:18 PDT
WK2 failures:
http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r150555%20(9117)/results.html
Jer Noble
Comment 26
2013-05-22 18:53:21 PDT
(In reply to
comment #24
)
> (From update of
attachment 202259
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=202259&action=review
> > > Source/WebCore/html/HTMLMediaElement.cpp:190 > > +static const size_t kPreferredLowPowerBufferSize = 4098; > > Where is this used?
It was unused. I deleted this line before committing.
Dean Jackson
Comment 27
2013-05-22 20:37:16 PDT
This broke about 30 media tests so we had to roll it out:
r150561
Jer Noble
Comment 28
2013-05-24 11:54:45 PDT
Committed
r150651
: <
http://trac.webkit.org/changeset/150651
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug