Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2411553003:
Notify WebMediaPlayer when its ancestor enters/exists fullscreen. (Closed)
|
Created:
4 years, 2 months ago by xjz Modified:
4 years, 1 month ago CC:
blink-reviews, blink-reviews-api_chromium.org, chromium-reviews, dglazkov+blink, eric.carlson_apple.com, feature-media-reviews_chromium.org, kinuko+watch, mlamouri+watch-blink_chromium.org, Srirama, RyanS Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionNotify WebMediaPlayer when its ancestor enters/exists fullscreen.
Inform the WebMediaPlayer when its ancestor enters fullscreen and
the video covers most of the window.
BUG=643964
Patch Set 1 #
Total comments: 9
Patch Set 2 : Rebased. Add FullscreenObserver. #
Messages
Total messages: 45 (19 generated)
Description was changed from ========== Notify WebMediaPlayer when its ancestor enters/exists fullscreen. Inform the WebmediaPlayer when its ancestor enters full screen and the video covers most of the window. BUG=643964 ========== to ========== Notify WebMediaPlayer when its ancestor enters/exists fullscreen. Inform the WebmediaPlayer when its ancestor enters full screen and the video covers most of the window. BUG=643964 ==========
xjz@chromium.org changed reviewers: + foolip@chromium.org, xhwang@chromium.org
xjz@chromium.org changed reviewers: + esprehn@chromium.org
This CL just includes the full screen related changes from original one: https://codereview.chromium.org/2389473002/ PTAL. foolip@: Now add the size check instead of notifying every video descendants. But I am not sure if the check works for common websites. It currently seems work on most web sites, but not YouTube. Can you PTAL? Thanks!
https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp (right): https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp:800: mediaPlayer->enteredFullscreen(); Does the case where the media element itself is the fullscreen element need to be handled differently? https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp:803: htmlMediaElement.document().domWindow()->innerWidth() * 0.9 && This check could pass for two videos where one obscures the other. I fear I may still not understand precisely what is hoped for, is the user experience that they are casting the whole tab, and if the video gets big enough, this logic kicks in and just the video is cast instead? This definitely has the structure of an intervention, of trying to fix the user experience for pages that could in principle do the right thing themselves (right?) but aren't doing so. https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/web/FullscreenController.cpp (right): https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/web/FullscreenController.cpp:141: videoElement.webMediaPlayer()->exitFullWindow(); Test and comment somewhere on what happens if the video element is removed from the document. That won't exit fullscreen, but it should change the cast mode back to plain tab mirroring.
Description was changed from ========== Notify WebMediaPlayer when its ancestor enters/exists fullscreen. Inform the WebmediaPlayer when its ancestor enters full screen and the video covers most of the window. BUG=643964 ========== to ========== Notify WebMediaPlayer when its ancestor enters/exists fullscreen. Inform the WebmediaPlayer when its ancestor enters full screen and the video covers most of the window. BUG=643964 ==========
Description was changed from ========== Notify WebMediaPlayer when its ancestor enters/exists fullscreen. Inform the WebmediaPlayer when its ancestor enters full screen and the video covers most of the window. BUG=643964 ========== to ========== Notify WebMediaPlayer when its ancestor enters/exists fullscreen. Inform the WebMediaPlayer when its ancestor enters full screen and the video covers most of the window. BUG=643964 ==========
Description was changed from ========== Notify WebMediaPlayer when its ancestor enters/exists fullscreen. Inform the WebMediaPlayer when its ancestor enters full screen and the video covers most of the window. BUG=643964 ========== to ========== Notify WebMediaPlayer when its ancestor enters/exists fullscreen. Inform the WebMediaPlayer when its ancestor enters fullscreen and the video covers most of the window. BUG=643964 ==========
Since we are talking about a heuristic here, is it already possible for web pages to do exactly the thing they want without the heuristic? In other words, can a page known that it's being cast, and can it say "cast just this video element instead" or "never cast just this video element"? I'm pretty sure the answer is no, which means that if we ship just the heuristic, sites may start having to do very silly things in cases where the heuristic isn't quite right, which is unavoidable.
foolip@chromium.org changed reviewers: + ojan@chromium.org
+ojan@, since I think this some of the features of an intervention, and ojan@ has also recently had much fun in media land with autoplay.
Thanks foolip@. PTAL https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp (right): https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp:800: mediaPlayer->enteredFullscreen(); On 2016/10/11 09:37:13, foolip wrote: > Does the case where the media element itself is the fullscreen element need to > be handled differently? It's currently handled same for Media Remoting. https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp:803: htmlMediaElement.document().domWindow()->innerWidth() * 0.9 && On 2016/10/11 09:37:13, foolip wrote: > This check could pass for two videos where one obscures the other. Yes, it could be. I think this case is not common use case, right? If it happens, only one video will be successfully casted to TV, the other one is still rendered locally. I fear I may > still not understand precisely what is hoped for, is the user experience that > they are casting the whole tab, and if the video gets big enough, this logic > kicks in and just the video is cast instead? Yes, if during mirroring, user clicks full screen button and video gets big enough, we'll swith to media remoting. We think this scenario indicates that user is only interested in the video playing back on remote devices (TV). So we'll stop local rendering, capturing, re-encoding, and just send the original video stream to ChromeCast to render and play on TV, which can save resources and improve mirroring quality. Does that make sense? > > This definitely has the structure of an intervention, of trying to fix the user > experience for pages that could in principle do the right thing themselves > (right?) but aren't doing so. I agree that there might be cases that don't work quite well. But we now aim at common use cases. And the remoting mode is easily to switch back to mirroring mode by exiting full screen. https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/web/FullscreenController.cpp (right): https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/web/FullscreenController.cpp:141: videoElement.webMediaPlayer()->exitFullWindow(); On 2016/10/11 09:37:13, foolip wrote: > Test and comment somewhere on what happens if the video element is removed from > the document. That won't exit fullscreen, but it should change the cast mode > back to plain tab mirroring. The WebMediaPlayer get destroyed and the romoting session stopped if the video element is removed from the document. One media remoting session is only with one HTMLMediaElement. Other kind of contents will be always rendered locally. There might be performance issue here as esprehn@ commented. If the video element is removed from the document, or if there is no video element called enterFullWindow(), all video descendants will be looked through.
On 2016/10/11 09:42:39, foolip wrote: > Since we are talking about a heuristic here, is it already possible for web > pages to do exactly the thing they want without the heuristic? In other words, > can a page known that it's being cast, and can it say "cast just this video > element instead" or "never cast just this video element"? I'm pretty sure the > answer is no, which means that if we ship just the heuristic, sites may start > having to do very silly things in cases where the heuristic isn't quite right, > which is unavoidable. Yes, you are right. There could be sites doing silly things with this logic. Currently we only aim at common use cases or common sites. And user could always easily switch back to mirroring by exiting the fullscreen mode. In future, we are going to develop the "sticky tab" feature, and remember the user's behavior, so that we can blacklist some sites from media remoting.
On 2016/10/11 09:43:23, foolip wrote: > +ojan@, since I think this some of the features of an intervention, and ojan@ > has also recently had much fun in media land with autoplay. To provide a brief context for this change: We are building a way to remote media content directly to a remote device (such as a Chromecast) for playback. (This is instead of playing back locally and then using tab mirroring to capture/re-encode before sending to the remote device.) The change in this CL is to notify WebMediaPlayerImpl that user clicks full screen button, and the video covers most of the tab window. This is used to indicate an "immersive" playback scenario, i.e., user is only/mostly interested in the video playback.
I really don't want to add this whole document traversing code. I feel like part of the problem here is that you're trying to implicitly detect video content which should be casted inside what would otherwise be full screen content. If you want to do that you need to keep track of all of the video elements in the entire page in a set and just traverse that instead. I'd much prefer a real API though, trying to detect this without the involvement of the page doesn't seem good, we're going to get it randomly wrong (ex. two videos). It'd be better if we shipped a real web platform API that let pages tell us what content to cast. https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/web/FullscreenController.cpp (right): https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/web/FullscreenController.cpp:92: Traversal<HTMLVideoElement>::descendantsOf(*element)) { I still don't think we should land this. It requires traversing the entire subtree of the full screen element, which can be a huge section of the page, for example it's the entire presentation in Google Slides.
https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp (right): https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp:800: mediaPlayer->enteredFullscreen(); On 2016/10/11 19:27:25, xjz wrote: > On 2016/10/11 09:37:13, foolip wrote: > > Does the case where the media element itself is the fullscreen element need to > > be handled differently? > > It's currently handled same for Media Remoting. Yes, but wouldn't it be covered by any heuristic used for when the fullscreen element is the parent? Doing that would increase confidence that the heuristic works, as breaking it would break the common case too.
On 2016/10/11 21:40:04, esprehn wrote: > I really don't want to add this whole document traversing code. I feel like part > of the problem here is that you're trying to implicitly detect video content > which should be casted inside what would otherwise be full screen content. If > you want to do that you need to keep track of all of the video elements in the > entire page in a set and just traverse that instead. I'd much prefer a real API > though, trying to detect this without the involvement of the page doesn't seem > good, we're going to get it randomly wrong (ex. two videos). It'd be better if > we shipped a real web platform API that let pages tell us what content to cast. This is essentially my view as well. A heuristic to enter a mode that the page can't explicitly request (other than changing the layout) makes the platform less predictable for web developers, although it can make good sense from a product/user perspective. I suspect that the real API in question here is https://w3c.github.io/remote-playback/ but the implementation isn't in terms of that API. Could it be, or is the thing that WebMediaPlayer does more powerful or another kind of casting altogether?
foolip@chromium.org changed reviewers: + avayvod@chromium.org, mlamouri@chromium.org
mlamouri@ avayvod@ ^^^
On 2016/10/12 at 11:59:46, foolip wrote: > mlamouri@ avayvod@ ^^^ AFAIK, media remoting is currently implemented as an optimization of tab mirroring - if the most of the tab being mirrored is a video, it could be more optimal to just send the encoded video data directly to your TV (given it supports decoding) vs decoding it on the sender, encoding the whole tab and sending the new stream to the TV. This is the optimization for the long tail of sites that don't use the available APIs (Presentation, Remote Playback, even the Cast Web SDK). Can we detect if the video occupying >90% of the screen is not obscured by any other element to eliminate the two videos case? Can we use an observer pattern for FullscreenController so video elements could subscribe and use (hopefully) linear search to check if it was their parent that's fullscreened?
https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp (right): https://codereview.chromium.org/2411553003/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp:800: mediaPlayer->enteredFullscreen(); On 2016/10/12 11:51:42, foolip wrote: > On 2016/10/11 19:27:25, xjz wrote: > > On 2016/10/11 09:37:13, foolip wrote: > > > Does the case where the media element itself is the fullscreen element need > to > > > be handled differently? > > > > It's currently handled same for Media Remoting. > > Yes, but wouldn't it be covered by any heuristic used for when the fullscreen > element is the parent? Doing that would increase confidence that the heuristic > works, as breaking it would break the common case too. The heuristic used for when the fullscreen element is the parent is to check whether the video occupies most of the tab window. If the media element itself is the fullscreen element, it should cover the full screen (or window if full-screen-in-tab), right? So I don't think we need to check that for this case, unless we want to confirm that the heuristic works on this case.
On 2016/10/12 11:59:17, foolip wrote: > On 2016/10/11 21:40:04, esprehn wrote: > > I really don't want to add this whole document traversing code. I feel like > part > > of the problem here is that you're trying to implicitly detect video content > > which should be casted inside what would otherwise be full screen content. If > > you want to do that you need to keep track of all of the video elements in the > > entire page in a set and just traverse that instead. I'd much prefer a real > API > > though, trying to detect this without the involvement of the page doesn't seem > > good, we're going to get it randomly wrong (ex. two videos). It'd be better if > > we shipped a real web platform API that let pages tell us what content to > cast. > > This is essentially my view as well. A heuristic to enter a mode that the page > can't explicitly request (other than changing the layout) makes the platform > less predictable for web developers, although it can make good sense from a > product/user perspective. > > I suspect that the real API in question here is > https://w3c.github.io/remote-playback/ but the implementation isn't in terms of > that API. Could it be, or is the thing that WebMediaPlayer does more powerful or > another kind of casting altogether? No, we are not implementing the Remote Playback API. As avayvod@ commented, Media Remoting is an optimization of tab mirroring. The changes in this CL are just trying to detect the scenario that we could do this optimization and improve the mirroring quality.
On 2016/10/12 14:30:59, whywhat wrote: > On 2016/10/12 at 11:59:46, foolip wrote: > > mlamouri@ avayvod@ ^^^ > > AFAIK, media remoting is currently implemented as an optimization of tab > mirroring - if the most of the tab being mirrored is a video, it could be more > optimal to just send the encoded video data directly to your TV (given it > supports decoding) vs decoding it on the sender, encoding the whole tab and > sending the new stream to the TV. This is the optimization for the long tail of > sites that don't use the available APIs (Presentation, Remote Playback, even the > Cast Web SDK). Correct! > > Can we detect if the video occupying >90% of the screen is not obscured by any > other element to eliminate the two videos case? Good point. Can we know this for a HTMLMediaElement? +foolip@ > > Can we use an observer pattern for FullscreenController so video elements could > subscribe and use (hopefully) linear search to check if it was their parent > that's fullscreened? In this way, all video elements will get notified from FullscreenController, and each video element need to check if it is a descendant of the fullscreen element. Will this have better performance than notifying all video descendants?
apacible@chromium.org changed reviewers: + apacible@chromium.org
On 2016/10/12 at 17:43:50, xjz wrote: > On 2016/10/12 14:30:59, whywhat wrote: > > On 2016/10/12 at 11:59:46, foolip wrote: > > > mlamouri@ avayvod@ ^^^ > > > > AFAIK, media remoting is currently implemented as an optimization of tab > > mirroring - if the most of the tab being mirrored is a video, it could be more > > optimal to just send the encoded video data directly to your TV (given it > > supports decoding) vs decoding it on the sender, encoding the whole tab and > > sending the new stream to the TV. This is the optimization for the long tail of > > sites that don't use the available APIs (Presentation, Remote Playback, even the > > Cast Web SDK). > Correct! > > > > Can we detect if the video occupying >90% of the screen is not obscured by any > > other element to eliminate the two videos case? > Good point. Can we know this for a HTMLMediaElement? +foolip@ > > > > Can we use an observer pattern for FullscreenController so video elements could > > subscribe and use (hopefully) linear search to check if it was their parent > > that's fullscreened? > In this way, all video elements will get notified from FullscreenController, and > each video element need to check if it is a descendant of the fullscreen element. > Will this have better performance than notifying all video descendants? Well, in some bad corner cases traversing all the parent nodes of each video element might traverse the whole DOM (imagine a page with multiple divs, each containing a video - when one div goes fullscreen, traversing its subtree would be much faster than checking each video element). On the other hand, imagine a complex page with one video somewhere that goes fullscreen with the root div - then traversing the video's parents would be faster. Unless you don't need to check if the video element is a descendant of the fullscreen element - can't you just check its size and visibility to trigger the optimization? Also, do you apply the optimization only when the video is playing? That could reduce the number of videos to check significantly. Maybe we could mark each element with a bit indicating if it contains a video in its subtree - setting this would be fast whenever a video element is added/removed - then you could significantly reduce the cost of subtree traversal for the fullscreen element. But that's a trickier change :) I wonder if we don't find the best solution due to potential worst cases for any of those, could we do a Finch experiment on 1% of Beta users each, for example. Might be that in practice one solution is always better.
Let's just keep a set of video elements and traverse up the tree. Video elements a very heavy weight, pages rarely have many of them. Also note that the depth of a typical page is quite small, rarely above 32 deep, while the total weight of a page is usually several thousand elements. -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Let's just keep a set of video elements and traverse up the tree. Video elements a very heavy weight, pages rarely have many of them. Also note that the depth of a typical page is quite small, rarely above 32 deep, while the total weight of a page is usually several thousand elements. -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
Regarding the feature, I think we should have a discussion outside of this codereview about the interventions process to apply here. I agree with Philip that this falls under the general interventions umbrella. Can you setup a meeting with the appropriate media folks and me+rschoen? Also, this is changing web exposed behavior, so should go through the usual intent to implement/ship process as well. Was there an intent email sent to blink-dev? I couldn't find one. Regarding the code here, I have a bit of a crazy idea... We already track video visibility with an IntersectionObserver via ElementVisibilityObserver. ElementVisibilityObserver could expose the intersection rect for the video with the viewport. With that information, you can know cheaply for each video whether it covers the viewport. Then we just need to find the visible videos that are large enough and walk up the ancestor chain for those to ensure they're under the fullscreened element. esprehn, wdyt?
On 2016/10/13 at 04:26:53, ojan wrote: > Regarding the feature, I think we should have a discussion outside of this codereview about the interventions process to apply here. I agree with Philip that this falls under the general interventions umbrella. Can you setup a meeting with the appropriate media folks and me+rschoen? Also, this is changing web exposed behavior, so should go through the usual intent to implement/ship process as well. Was there an intent email sent to blink-dev? I couldn't find one. I don't think this changes anything web exposed - it only switches the underlying media pipeline from the one that renders video frames to a local surface to the one that renders on a TV. The page can't really observe it in any way. > > Regarding the code here, I have a bit of a crazy idea... > > We already track video visibility with an IntersectionObserver via ElementVisibilityObserver. ElementVisibilityObserver could expose the intersection rect for the video with the viewport. With that information, you can know cheaply for each video whether it covers the viewport. Then we just need to find the visible videos that are large enough and walk up the ancestor chain for those to ensure they're under the fullscreened element. esprehn, wdyt?
xjz@chromium.org changed reviewers: + miu@chromium.org
The CQ bit was checked by xjz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by xjz@chromium.org to run a CQ dry run
Patchset #2 (id:20001) has been deleted
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks all for comments/suggestions! PS#2 adds a FullscreenObserver as avayvod@ suggested. Each HTMLMediaElement is registered as a FullscreenObserver and can get notified when an element enters/exits full screen. PTAL.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
On 2016/10/12 17:42:05, xjz wrote: > On 2016/10/12 11:59:17, foolip wrote: > > On 2016/10/11 21:40:04, esprehn wrote: > > > I really don't want to add this whole document traversing code. I feel like > > part > > > of the problem here is that you're trying to implicitly detect video content > > > which should be casted inside what would otherwise be full screen content. > If > > > you want to do that you need to keep track of all of the video elements in > the > > > entire page in a set and just traverse that instead. I'd much prefer a real > > API > > > though, trying to detect this without the involvement of the page doesn't > seem > > > good, we're going to get it randomly wrong (ex. two videos). It'd be better > if > > > we shipped a real web platform API that let pages tell us what content to > > cast. > > > > This is essentially my view as well. A heuristic to enter a mode that the page > > can't explicitly request (other than changing the layout) makes the platform > > less predictable for web developers, although it can make good sense from a > > product/user perspective. > > > > I suspect that the real API in question here is > > https://w3c.github.io/remote-playback/ but the implementation isn't in terms > of > > that API. Could it be, or is the thing that WebMediaPlayer does more powerful > or > > another kind of casting altogether? > > No, we are not implementing the Remote Playback API. As avayvod@ commented, > Media Remoting > is an optimization of tab mirroring. The changes in this CL are just trying to > detect the > scenario that we could do this optimization and improve the mirroring quality. It is more than an optimization, with the 90% heuristic or anything similar, it changes what happens in more ways than improving quality. To do this only when the video exactly covers the full viewport and there are no controls or captions would be unproblematic I think, barring bugs there would be no reason for sites to want to opt out of that behavior.
On 2016/10/13 15:22:12, whywhat wrote: > On 2016/10/13 at 04:26:53, ojan wrote: > > Regarding the feature, I think we should have a discussion outside of this > codereview about the interventions process to apply here. I agree with Philip > that this falls under the general interventions umbrella. Can you setup a > meeting with the appropriate media folks and me+rschoen? Also, this is changing > web exposed behavior, so should go through the usual intent to implement/ship > process as well. Was there an intent email sent to blink-dev? I couldn't find > one. > > I don't think this changes anything web exposed - it only switches the > underlying media pipeline from the one that renders video frames to a local > surface to the one that renders on a TV. The page can't really observe it in any > way. It's true that the page can't detect it (modulo bugs) but the user can and it might have side effects like dropping captions, so users and/or the web developer might want to influence when it kicks in. And if the behavior can't be expressed in terms of things like IntersectionObserver and Remote Playback API, that could suggest that something is missing from the platform. That could be OK in the interim, but understanding what that something is would be good. I think a meeting would be a better way to figure out what to do with this. No need to invite me if time zones look hard, though.
On 2016/10/18 at 13:10:31, foolip wrote: > On 2016/10/13 15:22:12, whywhat wrote: > > On 2016/10/13 at 04:26:53, ojan wrote: > > > Regarding the feature, I think we should have a discussion outside of this > > codereview about the interventions process to apply here. I agree with Philip > > that this falls under the general interventions umbrella. Can you setup a > > meeting with the appropriate media folks and me+rschoen? Also, this is changing > > web exposed behavior, so should go through the usual intent to implement/ship > > process as well. Was there an intent email sent to blink-dev? I couldn't find > > one. > > > > I don't think this changes anything web exposed - it only switches the > > underlying media pipeline from the one that renders video frames to a local > > surface to the one that renders on a TV. The page can't really observe it in any > > way. > > It's true that the page can't detect it (modulo bugs) but the user can and it might have side effects like dropping captions, so users and/or the web developer might want to influence when it kicks in. And if the behavior can't be expressed in terms of things like IntersectionObserver and Remote Playback API, that could suggest that something is missing from the platform. That could be OK in the interim, but understanding what that something is would be good. > > I think a meeting would be a better way to figure out what to do with this. No need to invite me if time zones look hard, though. Mostly just +1'ing here, but hopefully this gives a bit more color to it... Being observable and web exposed are different things. For example, incorrectly painting blue borders as red is not observable, but it's certainly web exposed, right? My definition of web exposed here is that it's something web authors have to consider when making web pages. In this case, I think the practical changes I would want is giving authors control over what happens (e.g. a new attribute on <video>). It's OK to have a default behavior that has a heuristic, but one of the interventions principles is that heuristics should be under the author's control unless there's a strong reason not to. And once we expose control over the behavior, it will definitely need an intent email. :) That said, I think codereview is not the best place to discuss high-level product things like this, which is why I was asking for a meeting. We could start over email if you prefer that.
On 2016/10/18 23:46:49, ojan wrote: > That said, I think codereview is not the best place to discuss high-level > product things like this, which is why I was asking for a meeting. We could > start over email if you prefer that. I just sent an e-mail to everyone to clear up some issues/misconceptions in the code review discussion here. Hopefully, you'll all agree the scoping here is mainly browser-oriented (rather than web-platform-oriented); and that we can proceed with this code review. ;-)
On 2016/10/19 05:36:27, miu wrote: > On 2016/10/18 23:46:49, ojan wrote: > > That said, I think codereview is not the best place to discuss high-level > > product things like this, which is why I was asking for a meeting. We could > > start over email if you prefer that. > > I just sent an e-mail to everyone to clear up some issues/misconceptions in the > code review discussion here. Hopefully, you'll all agree the scoping here is > mainly browser-oriented (rather than web-platform-oriented); and that we can > proceed with this code review. ;-) As per discussion, we are not going to use the "Entered/ExitedFullscreen" event as a signal for turning Media Remoting on/off. Instead, we may use the "90% viewport intersection" heuristic. So close this CL for now. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
