Prevent AudioSession category from moving out of PlayAndRecord too quickly

• Component: WebRTC
• Status: RESOLVED FIXED

[youenn fablet]

Prevent AudioSession category to move out of PlayAndRecord to quickly

[youenn fablet]

<rdar://81997024>

[youenn fablet]

Created attachment 435954 [details]
Patch

[youenn fablet]

<rdar://81283764>

[youenn fablet]

Created attachment 435955 [details]
Patch

[youenn fablet]

We could remove individual MediaStreamTrack registration by making sure the Document media state is updated synchronously. I left this as a follow-up to make this patch small.

[youenn fablet]

Created attachment 435971 [details]
Patch

[youenn fablet]

Created attachment 435976 [details]
Patch

[youenn fablet]

Created attachment 435981 [details]
Patch

[youenn fablet]

(In reply to youenn fablet from comment #5)
> We could remove individual MediaStreamTrack registration by making sure the
> Document media state is updated synchronously. I left this as a follow-up to
> make this patch small.

Initial version was using Document to ensure PlayAndRecord sticks.
New version is moving the logic inside MediaSessionManagerCocoa which is more appropriate.

[Eric Carlson]

Comment on attachment 435981 [details]
Patch

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

> Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:154
> +    if (AudioSession::sharedSession().category() == AudioSession::CategoryType::PlayAndRecord && isPlayingAudio)
> +        category = AudioSession::CategoryType::PlayAndRecord;

Nit: it would be easier to understand if this was first:

    if (captureCount  || AudioSession::sharedSession().category() == AudioSession::CategoryType::PlayAndRecord && isPlayingAudio)
        category = AudioSession::CategoryType::PlayAndRecord;
    else if (...

> LayoutTests/http/tests/media/media-stream/audio-capture-and-category.https.html:45
> +     assert_equals(internals.audioSessionCategory(), "PlayAndRecord", "category when capturing");

Nit: it would be good to make this assert message different from the one above so if the test fails we can tell which test was the problem.

[youenn fablet]

Created attachment 435995 [details]
Patch

[youenn fablet]

Created attachment 435996 [details]
Patch

[youenn fablet]

> > Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:154
> > +    if (AudioSession::sharedSession().category() == AudioSession::CategoryType::PlayAndRecord && isPlayingAudio)
> > +        category = AudioSession::CategoryType::PlayAndRecord;
> 
> Nit: it would be easier to understand if this was first:

Done

> > LayoutTests/http/tests/media/media-stream/audio-capture-and-category.https.html:45
> > +     assert_equals(internals.audioSessionCategory(), "PlayAndRecord", "category when capturing");
> 
> Nit: it would be good to make this assert message different from the one
> above so if the test fails we can tell which test was the problem.

Done as well.

[Eric Carlson]

Comment on attachment 435996 [details]
Patch

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

r=me once the bots are happy

> LayoutTests/http/tests/media/media-stream/audio-capture-and-category.https.html:46
> +     assert_equals(internals.audioSessionCategory(), "PlayAndRecord", "category when capturing");

Maybe "category when capturing and playing audio"

> LayoutTests/http/tests/media/media-stream/audio-capture-and-category.https.html:48
> +     stream.getAudioTracks()[0].stop();

As we discussed, it would be good to either comment here or use a url src for the audio element to make it obvious that audio will continue to play when the capture track is stopped.

> LayoutTests/http/tests/media/media-stream/audio-capture-and-category.https.html:51
> +     assert_equals(internals.audioSessionCategory(), "PlayAndRecord", "category when document did capture but is not capturing anymore");

Maybe "category when capture has stopped but audio continues to play"

[youenn fablet]

Created attachment 436008 [details]
Patch for landing

[EWS]

Committed r281367 (240782@main): <https://commits.webkit.org/240782@main>

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