Bug 209789 - [iPad] Use AVAudioSession to detect AirPlay route changes
Summary: [iPad] Use AVAudioSession to detect AirPlay route changes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-30 16:49 PDT by Eric Carlson
Modified: 2020-03-31 12:40 PDT (History)
10 users (show)

See Also:

Attachments
Patch (29.48 KB, patch)
2020-03-30 17:08 PDT, Eric Carlson 		no flags Details | Formatted Diff | Diff
Patch (31.07 KB, patch)
2020-03-31 09:11 PDT, Eric Carlson 		no flags Details | Formatted Diff | Diff
Patch (30.38 KB, patch)
2020-03-31 09:55 PDT, Eric Carlson 		jer.noble: review+
Details | Formatted Diff | Diff
Patch for landing (31.80 KB, patch)
2020-03-31 11:08 PDT, Eric Carlson 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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].