New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 669081 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Fullscreen ratio in Blob URL video is wrong for vertical videos

Reported by michael....@myinterview.com, Nov 28 2016

Issue description

Steps to reproduce the problem:
1. Open https://jsfiddle.net/onposg5k/2/ on Chrome 54 in Android
2. Click Upload a file and record a camera video on vertical orientation
3. Play it and put it in fullscreen

What is the expected behavior?
Normal ratio

What went wrong?
Somewhat Chrome swaps the height and width and completely distorts the video.

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 54.0.2840.99  Channel: stable
OS Version: 6.0.1
Flash Version: Shockwave Flash 23.0 r0

Works in stock browser.
 
Screenshot_20161128-184148.png
955 KB View Download
Labels: Needs-Feedback
I can't reproduce locally with my devices (several different models). Can you provide more details such as device model?
Sure,
my device is OnePlusOne on Android 6.0.1

Also reproduced Nexus 6P on Android 6.0.1
Also reproduced on Nexus 5 on Android 6.0.1

Added a video of the problem (sorry for bad quality)
WhatsApp Video 2016-11-29 at 15.58.40.mp4
5.2 MB View Download
Also tried with last version of Google Chrome Beta which gives the same result...
Components: Blink>Fullscreen
Labels: -Arch-x86_64
Status: Untriaged (was: Unconfirmed)
OK, just realized I need to go fullscreen. Sorry I didn't noticed it in the beginning.
Cc: dalecur...@chromium.org
Dale, can you take a look?
Cc: liber...@chromium.org
=>liberato -- I'd guess we're not accounting for video rotation on our SurfaceViews. Not sure how to do that.
Cc: w...@chromium.org
i don't know how to do that either.  i'm not sure that one can, though SurfaceFlinger does support it.  not sure if we can set it ourselves, though.

we might need to turn off SV in that case.
Components: -Blink>Fullscreen -Blink>Media Internals>Media>Video
Moving this to Internals>Media based on comment #8.
Hi, any update on this? Thanks :)
Cc: -liber...@chromium.org
Owner: liber...@chromium.org
Status: Assigned (was: Untriaged)
liberato@, can you take this bug?
Cc: mlamouri@chromium.org
Do you have any workaround in the meanwhile that would make it work?
Thanks
Hi, any update on the issue?
Thanks
sorry, forgot to update this.  i found no way to handle this via public android APIs.  

i'm trying to figure out if we can turn off overlay promotion in this case.
turns out to be much easier than i originally pictured.  https://codereview.chromium.org/2611783002
Project Member

Comment 17 by bugdroid1@chromium.org, Jan 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2fd111be3c997d2ab9ce570d9076f3ec0da7a93e

commit 2fd111be3c997d2ab9ce570d9076f3ec0da7a93e
Author: liberato <liberato@chromium.org>
Date: Wed Jan 04 00:25:06 2017

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
Cr-Commit-Position: refs/heads/master@{#441264}

[modify] https://crrev.com/2fd111be3c997d2ab9ce570d9076f3ec0da7a93e/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/2fd111be3c997d2ab9ce570d9076f3ec0da7a93e/media/blink/webmediaplayer_impl.h

Status: Fixed (was: Assigned)
Great, thanks for your fix ! 
How can I track on which versions it will be implemented?
It should be included in 57 stable.  Canary will have it sooner, probably a day or two, if you want to verify the fix.  Be sure not to let canary update over cellular data automatically, since it is updated very frequently.

Sorry again about the delay.
Might be worth merging to M56?
Labels: Merge-Request-56
good point.
Project Member

Comment 23 by sheriffbot@chromium.org, Jan 6 2017

Labels: -Merge-Request-56 Hotlist-Merge-Approved Merge-Approved-56
Your change meets the bar and is auto-approved for M56. Please go ahead and merge the CL manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), gkihumba@(cros), bustamante@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 24 by sheriffbot@chromium.org, Jan 9 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 25 by bugdroid1@chromium.org, Jan 10 2017

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/04159658b3aba4c732bcd9332df6f98c40bc7f21

commit 04159658b3aba4c732bcd9332df6f98c40bc7f21
Author: liberato <liberato@chromium.org>
Date: Tue Jan 10 20:01:57 2017

[M56] Turn off video overlay on android for rotated video.

Merge to M56 of https://codereview.chromium.org/2611783002 .

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 
TBR=dalecurtis@chromium.org
NOPRESUBMIT=true
NOTRY=true

Review-Url: https://codereview.chromium.org/2625513002
Cr-Commit-Position: refs/branch-heads/2924@{#719}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/04159658b3aba4c732bcd9332df6f98c40bc7f21/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/04159658b3aba4c732bcd9332df6f98c40bc7f21/media/blink/webmediaplayer_impl.h

Status: Verified (was: Fixed)
Works fine as per expected behavior ,Verified in Latest M56 on Nexus 5x/ N and Nexus 6/ N.

Sign in to add a comment