Bug 209789

Summary: [iPad] Use AVAudioSession to detect AirPlay route changes
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, cdumez, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jer.noble, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
jer.noble: review+
Patch for landing none

Description Eric Carlson 2020-03-30 16:49:31 PDT 
Use AVAudioSession to detect AirPlay route changes
Comment 1 Eric Carlson 2020-03-30 16:49:45 PDT 
<rdar://problem/58065279>
Comment 2 Eric Carlson 2020-03-30 17:08:58 PDT 
Created attachment 394989 [details]
Patch
Comment 3 Eric Carlson 2020-03-31 09:11:00 PDT 
Created attachment 395046 [details]
Patch
Comment 4 Eric Carlson 2020-03-31 09:55:39 PDT 
Created attachment 395055 [details]
Patch
Comment 5 Jer Noble 2020-03-31 10:35:48 PDT 
Comment on attachment 395055 [details]
Patch

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

r=me with nits.

> Source/WebCore/platform/audio/ios/MediaSessionHelperIOS.mm:106
> +void activeAudioRouteDidChange(bool);
> +void activeVideoRouteDidChange();

Weird spacing here.

> Source/WebCore/platform/graphics/MediaPlaybackTarget.h:47
> +    virtual bool supportsAirPlayVideo() const = 0;

It's kind of weird to have the term "AirPlay" here in a cross-port header.  Could we change this to "supportsRemoteVideoPlayback" or something?
Comment 6 Eric Carlson 2020-03-31 11:06:59 PDT 
Comment on attachment 395055 [details]
Patch

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

Thanks for the review!

>> Source/WebCore/platform/audio/ios/MediaSessionHelperIOS.mm:106
>> +void activeVideoRouteDidChange();
> 
> Weird spacing here.

Fixed.

>> Source/WebCore/platform/graphics/MediaPlaybackTarget.h:47
>> +    virtual bool supportsAirPlayVideo() const = 0;
> 
> It's kind of weird to have the term "AirPlay" here in a cross-port header.  Could we change this to "supportsRemoteVideoPlayback" or something?

Good point, fixed.
Comment 7 Eric Carlson 2020-03-31 11:08:24 PDT 
Created attachment 395070 [details]
Patch for landing
Comment 8 EWS 2020-03-31 12:40:11 PDT 
Committed r259303: <https://trac.webkit.org/changeset/259303>

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