Prevent AudioSession category to move out of PlayAndRecord to quickly
<rdar://81997024>
Created attachment 435954 [details] Patch
<rdar://81283764>
Created attachment 435955 [details] Patch
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.
Created attachment 435971 [details] Patch
Created attachment 435976 [details] Patch
Created attachment 435981 [details] Patch
(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.
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.
Created attachment 435995 [details] Patch
Created attachment 435996 [details] Patch
> > 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.
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"
Created attachment 436008 [details] Patch for landing
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].