Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2611783002:
Turn off video overlay on android for rotated video. (Closed)
|
Created:
3 years, 11 months ago by liberato (no reviews please) Modified:
3 years, 11 months ago Reviewers:
watk CC:
chromium-reviews, feature-media-reviews_chromium.org, xjz+watch_chromium.org, miu+watch_chromium.org, apacible+watch_chromium.org, erickung+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionTurn off video overlay on android for rotated video.
Don't enable SurfaceView overlays for video if the metadata says
that the video is rotated, since SurfaceView won't rotate it.
Also switch out of overlay mode if the metadata changes.
This also changes the 'exit full screen' logic to check if an
overlay is in use to decide if it needs to turn them off.
BUG=669081
Committed: https://crrev.com/2fd111be3c997d2ab9ce570d9076f3ec0da7a93e
Cr-Commit-Position: refs/heads/master@{#441264}
Patch Set 1 #
Total comments: 6
Patch Set 2 : fixed |force_video_overlays_| #
Messages
Total messages: 19 (13 generated)
The CQ bit was checked by liberato@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...
Description was changed from ========== Turn off video overlay on android for rotated video. Don't enable SurfaceView overlays for video if the metadata says that the video is rotated, since SurfaceView won't rotate it. Also switch out of overlay mode if the metadata changes. This also changes the 'exit full screen' logic to check if an overlay is in use to decide if it needs to turn them off. BUG=669081 ========== to ========== Turn off video overlay on android for rotated video. Don't enable SurfaceView overlays for video if the metadata says that the video is rotated, since SurfaceView won't rotate it. Also switch out of overlay mode if the metadata changes. This also changes the 'exit full screen' logic to check if an overlay is in use to decide if it needs to turn them off. BUG=669081 ==========
liberato@chromium.org changed reviewers: + watk@chromium.org
https://codereview.chromium.org/2611783002/diff/1/media/blink/webmediaplayer_... File media/blink/webmediaplayer_impl.cc (right): https://codereview.chromium.org/2611783002/diff/1/media/blink/webmediaplayer_... media/blink/webmediaplayer_impl.cc:350: DoesOverlaySupportMetadata()) needs braces now https://codereview.chromium.org/2611783002/diff/1/media/blink/webmediaplayer_... media/blink/webmediaplayer_impl.cc:357: if (overlay_enabled_) I don't think we should do this when force_video_overlays_? https://codereview.chromium.org/2611783002/diff/1/media/blink/webmediaplayer_... media/blink/webmediaplayer_impl.cc:1163: DisableOverlay(); I don't think we should do this for force_video_overlays_ either.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by liberato@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...
thanks -fl https://codereview.chromium.org/2611783002/diff/1/media/blink/webmediaplayer_... File media/blink/webmediaplayer_impl.cc (right): https://codereview.chromium.org/2611783002/diff/1/media/blink/webmediaplayer_... media/blink/webmediaplayer_impl.cc:350: DoesOverlaySupportMetadata()) On 2017/01/03 20:33:03, watk wrote: > needs braces now Done. https://codereview.chromium.org/2611783002/diff/1/media/blink/webmediaplayer_... media/blink/webmediaplayer_impl.cc:357: if (overlay_enabled_) On 2017/01/03 20:33:02, watk wrote: > I don't think we should do this when force_video_overlays_? good catch. not sure why i thought that was okay. added a comment too. https://codereview.chromium.org/2611783002/diff/1/media/blink/webmediaplayer_... media/blink/webmediaplayer_impl.cc:1163: DisableOverlay(); On 2017/01/03 20:33:03, watk wrote: > I don't think we should do this for force_video_overlays_ either. yeah, though we're kinda going to be wrong either way. probably better to be wrong and keep the state machine working, though. Done, plus comment.
cool, lgtm!
The CQ bit was unchecked by liberato@chromium.org
The CQ bit was checked by liberato@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483485364753290,
"parent_rev": "bfea5ab532926434a36b52e812ec36d47af26c21", "commit_rev":
"c5c445024a3bf1486f02aa8172a7bf4bc0c18c1f"}
Message was sent while issue was closed.
Description was changed from ========== Turn off video overlay on android for rotated video. Don't enable SurfaceView overlays for video if the metadata says that the video is rotated, since SurfaceView won't rotate it. Also switch out of overlay mode if the metadata changes. This also changes the 'exit full screen' logic to check if an overlay is in use to decide if it needs to turn them off. BUG=669081 ========== to ========== Turn off video overlay on android for rotated video. Don't enable SurfaceView overlays for video if the metadata says that the video is rotated, since SurfaceView won't rotate it. Also switch out of overlay mode if the metadata changes. This also changes the 'exit full screen' logic to check if an overlay is in use to decide if it needs to turn them off. BUG=669081 Review-Url: https://codereview.chromium.org/2611783002 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Turn off video overlay on android for rotated video. Don't enable SurfaceView overlays for video if the metadata says that the video is rotated, since SurfaceView won't rotate it. Also switch out of overlay mode if the metadata changes. This also changes the 'exit full screen' logic to check if an overlay is in use to decide if it needs to turn them off. BUG=669081 Review-Url: https://codereview.chromium.org/2611783002 ========== to ========== Turn off video overlay on android for rotated video. Don't enable SurfaceView overlays for video if the metadata says that the video is rotated, since SurfaceView won't rotate it. Also switch out of overlay mode if the metadata changes. This also changes the 'exit full screen' logic to check if an overlay is in use to decide if it needs to turn them off. BUG=669081 Committed: https://crrev.com/2fd111be3c997d2ab9ce570d9076f3ec0da7a93e Cr-Commit-Position: refs/heads/master@{#441264} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/2fd111be3c997d2ab9ce570d9076f3ec0da7a93e Cr-Commit-Position: refs/heads/master@{#441264} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
