Bug 212970

Summary: BaseAudioSharedUnit should unmute its clients in case of suspension even if not having any audio unit
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebRTCAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, ews-watchlist, glenn, hta, jer.noble, philipj, sergio, tommyw, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description youenn fablet 2020-06-09 06:30:35 PDT 
BaseAudioSharedUnit should unmute its clients in case of suspension even if not having any audio unit
Comment 1 Radar WebKit Bug Importer 2020-06-09 06:31:30 PDT 
<rdar://problem/64161891>
Comment 2 youenn fablet 2020-06-09 06:41:52 PDT 
Created attachment 401437 [details]
Patch
Comment 3 Eric Carlson 2020-06-09 07:15:35 PDT 
Comment on attachment 401437 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        This will, in turn, make the BaseAudioSharedUnit to stop and no longer have any audio unit.

s/the BaseAudioSharedUnit to stop/the BaseAudioSharedUnit stop/

> Source/WebCore/platform/mediastream/mac/BaseAudioSharedUnit.cpp:179
> +    ASSERT(!m_producingCount);

Is this backwards?
Comment 4 youenn fablet 2020-06-09 08:36:57 PDT 
Comment on attachment 401437 [details]
Patch

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

>> Source/WebCore/platform/mediastream/mac/BaseAudioSharedUnit.cpp:179
>> +    ASSERT(!m_producingCount);
> 
> Is this backwards?

Given the current model, m_producingCount should be zero.
I guess I could remove the if(m_producingCount) check.
Comment 5 youenn fablet 2020-06-09 08:37:21 PDT 
Created attachment 401442 [details]
Patch
Comment 6 Eric Carlson 2020-06-09 08:51:14 PDT 
Comment on attachment 401442 [details]
Patch

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

> Source/WebCore/platform/mediastream/mac/BaseAudioSharedUnit.cpp:180
>      if (m_producingCount) {

This is confusing given the assert.
Comment 7 youenn fablet 2020-06-09 11:03:51 PDT 
(In reply to Eric Carlson from comment #6)
> Comment on attachment 401442 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=401442&action=review
> 
> > Source/WebCore/platform/mediastream/mac/BaseAudioSharedUnit.cpp:180
> >      if (m_producingCount) {
> 
> This is confusing given the assert.

Will remove it in a follow-up
Comment 8 EWS 2020-06-09 11:16:13 PDT 
Committed r262798: <https://trac.webkit.org/changeset/262798>

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