Summary: | Introduce a mock implementation of CoreAudioSharedUnit | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||||||
Component: | WebRTC | Assignee: | youenn fablet <youennf> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | aakash_jain, ap, commit-queue, eric.carlson, ews-watchlist, glenn, hta, jer.noble, philipj, sergio, tommyw, webkit-bot-watchers-bugzilla, webkit-bug-importer, youennf | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=179411 https://bugs.webkit.org/show_bug.cgi?id=204408 |
||||||||||||||
Attachments: |
|
Description
youenn fablet
2019-11-18 00:13:21 PST
Created attachment 383733 [details]
Patch
Created attachment 383817 [details]
Patch
Created attachment 383829 [details]
Patch
Comment on attachment 383829 [details] Patch Attachment 383829 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/13266071 New failing tests: imported/blink/fast/events/panScroll-crash.html Created attachment 383847 [details]
Archive of layout-test-results from ews211 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews211 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment on attachment 383847 [details]
Archive of layout-test-results from ews211 for win-future
Error unrelated
Comment on attachment 383829 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383829&action=review > Source/WebCore/platform/mediastream/mac/BaseAudioSharedUnit.h:71 > + virtual CapabilityValueOrRange sampleRateCapacities() const { return CapabilityValueOrRange(8000, 96000); } Nit: it is a bit strange to have this return a default value from the base class when other trivial methods (isProducingData, hasAudioUnit) do not. It would be cleaner to also make it pure virtual. > Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp:896 > + auto& unit = m_overrideUnit ? *m_overrideUnit : CoreAudioSharedUnit::singleton(); > if (unit.isSuspended()) if (this->unit().isSuspended()) > Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp:905 > + if (m_overrideUnit) > + m_overrideUnit->delaySamples(seconds); Maybe have the non-mock unit, or the base class, have an empty delaySamples method instead? Created attachment 383866 [details]
Patch for landing
(In reply to Eric Carlson from comment #7) > Comment on attachment 383829 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=383829&action=review > > > Source/WebCore/platform/mediastream/mac/BaseAudioSharedUnit.h:71 > > + virtual CapabilityValueOrRange sampleRateCapacities() const { return CapabilityValueOrRange(8000, 96000); } > > Nit: it is a bit strange to have this return a default value from the base > class when other trivial methods (isProducingData, hasAudioUnit) do not. It > would be cleaner to also make it pure virtual. OK > > > Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp:896 > > + auto& unit = m_overrideUnit ? *m_overrideUnit : CoreAudioSharedUnit::singleton(); > > if (unit.isSuspended()) > > if (this->unit().isSuspended()) Issue was constness, I added a unit() const version. > > Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp:905 > > + if (m_overrideUnit) > > + m_overrideUnit->delaySamples(seconds); > > Maybe have the non-mock unit, or the base class, have an empty delaySamples > method instead? It already has so I changed it to unit().delaySamples. Comment on attachment 383866 [details] Patch for landing Clearing flags on attachment: 383866 Committed r252660: <https://trac.webkit.org/changeset/252660> All reviewed patches have been landed. Closing bug. > Committed r252660: <https://trac.webkit.org/changeset/252660> This seems to have broken platform/ios/mediastream/audio-muted-in-background-tab.html Tracked in Bug 204408. |