| Summary: | [iOS] unable to airplay directly loaded fullscreen video | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Devin Rousso <hi> | ||||||||
| Component: | Media | Assignee: | Devin Rousso <hi> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | calvaris, cdumez, changseok, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, hi, jer.noble, peng.liu6, philipj, sergio, thorton, webkit-bug-importer, wenson_hsieh | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| URL: | https://devinrousso.com/demo/WebKit/stream_of_water.mp4 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Devin Rousso
2020-09-22 16:50:03 PDT
Created attachment 409426 [details]
Patch
This doesn't have any tests yet. I'm mainly uploading it to see what the bots think while I figure out how to test this (if it's even possible, as it only appears to reproduce on directly loaded videos).
Comment on attachment 409426 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409426&action=review > Source/WebCore/ChangeLog:18 > + is initally reported as "disabled" and then almost immediately changed to "enabled". Sounds like AVKit does not use the up-to-date value of the variable? Created attachment 409518 [details]
Patch
Re-approached the fix from another angle. Working on writing tests.
Comment on attachment 409426 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409426&action=review >> Source/WebCore/ChangeLog:18 >> + is initally reported as "disabled" and then almost immediately changed to "enabled". > > Sounds like AVKit does not use the up-to-date value of the variable? as it turns out, it's more like "WebKit did not propagate the new value if/when the value returned by `wirelessVideoPlaybackDisabled` changes to the UIProcess so that WebKit could call `-[WebAVPlayerController setAllowsExternalPlayback:]` again with the new value" :P please see attachment 409518 [details] for more details :) Comment on attachment 409518 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409518&action=review > Source/WebCore/html/HTMLMediaElement.cpp:5009 > + if (m_player) { Is this change needed? Comment on attachment 409518 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409518&action=review >> Source/WebCore/html/HTMLMediaElement.cpp:5009 >> + if (m_player) { > > Is this change needed? yes, as otherwise this function early-returns before the added code below is able to run I've tried looking at whether any of the other functions after this do in-fact require a `m_player` to exist, and I don't think they do, but this code is quite nested and intertwined so I'm not entirely sure. I figured that EWS would tell me if this was bad :) Comment on attachment 409518 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409518&action=review >>> Source/WebCore/html/HTMLMediaElement.cpp:5009 >>> + if (m_player) { >> >> Is this change needed? > > yes, as otherwise this function early-returns before the added code below is able to run > > I've tried looking at whether any of the other functions after this do in-fact require a `m_player` to exist, and I don't think they do, but this code is quite nested and intertwined so I'm not entirely sure. I figured that EWS would tell me if this was bad :) Hmm, I see! Sounds a little strange that m_player is nullptr but we need to notify observers regarding the media engine change. :-) Created attachment 409764 [details]
Patch
Comment on attachment 409764 [details]
Patch
Thanks for adding a test!
Committed r267709: <https://trac.webkit.org/changeset/267709> All reviewed patches have been landed. Closing bug and clearing flags on attachment 409764 [details]. |