Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scaling the video independently of the size of the SurfaceView #1428

Closed
martinbonnin opened this issue Apr 8, 2016 · 14 comments
Closed

Scaling the video independently of the size of the SurfaceView #1428

martinbonnin opened this issue Apr 8, 2016 · 14 comments

Comments

@martinbonnin
Copy link

Is it somehow possible to manually control how the video is scaled in a SurfaceView ? What I have in mind is having something like a letterbox format but that could switch to VIDEO_SCALING_MODE_SCALE_TO_FIT_WITH_CROPPING when the SurfaceView is collapsed.

I have read #1076 and https://code.google.com/p/android/issues/detail?id=37800 but none seem to provide a solution for letterboxing.

I guess I could workaround that by drawing myself in a GLSurfaceView using openGL but I'm unsure if this has battery/performance implications. Any idea how to do this ?

@martinbonnin
Copy link
Author

Some more thoughts:

  • For leterboxing, I can certainly use the LayoutParams and put a black view behind my SurfaceView or better, 2 black views on the sides to avoid overdrawing.
  • VIDEO_SCALING_MODE_SCALE_TO_FIT_WITH_CROPPING doesn't seem to be working on my Nexus 5X. No matter what I pass to MediaCodecVideoTrackRenderer constructor, the video is stretched to the exact size of the SurfaceView. Anyone seeing the same behaviour ?

@ojw28
Copy link
Contributor

ojw28 commented Apr 11, 2016

I don't think I really understand what the request is here. Please clarify?

To address the final point, I'm pretty sure VIDEO_SCALING_MODE_SCALE_TO_FIT_WITH_CROPPING works correctly. Note that if you're using AspectRatioFrameLayout or similar then the scaling mode isn't going to make a difference; the difference will only be apparent if the SurfaceView has a different aspect ratio to the video. Note also that burnt in letter-boxing (i.e. black bars that are actually part of the video) count as part of the video w.r.t scaling modes.

@martinbonnin
Copy link
Author

Hi ! Sorry for the confusion. There are 2 questions:

@ojw28
Copy link
Contributor

ojw28 commented Apr 12, 2016

Nope, you didn't miss something. It appears that setVideoScalingMode doesn't work from the place where we're calling it. It should work, as far as I can tell by looking at the MediaCodec documentation. I'll follow up with the Android platform team about that.

I found that if you move the call to setVideoScalingMode in MediaCodecVideoTrackRenderer to happen in onOutputFormatChanged, then in that case the call does work correctly. It's probably the case that we should move the call there, seeing as that works, although I'd like to confirm expected behavior with the platform team first. In the meantime, you can give that a try at least and verify that it works as expected!

@ojw28 ojw28 added bug and removed need more info labels Apr 12, 2016
@ojw28
Copy link
Contributor

ojw28 commented Apr 12, 2016

Marking as a bug to make sure we fix/workaround application of the scaling mode.

@brianwernick
Copy link

brianwernick commented Apr 15, 2016

If it helps either of you (or anyone else reaching this issue) I have implemented a Center_crop style (among other) scaling in brianwernick/ExoMedia#158 and fixed in https:/brianwernick/ExoMedia/pull/168/files.

In short I wait until the ExoPlayer informs us that the video size has changed, then wait for the TextureView to resize before applying the modified scaling

@martinbonnin
Copy link
Author

martinbonnin commented Apr 15, 2016

Hi @brianwernick, I don't really want to use a TextureView unless you have significant data showing that it does not hurt the battery too much (see #1084).

I was considering using a GLSurfaceView instead which should be a bit better since it can still use the hardware layers for composition. But even that still requires some additional frames copy if I understand correctly.

Having VIDEO_SCALING_MODE_SCALE_TO_FIT_WITH_CROPPING work would be the best solution, battery usage wise.

@brianwernick
Copy link

I agree that having the MediaCodec perform the work will be better performance/battery wise due to only having to do a single pass for each frame.

It does look like I skimmed over some information that was pertinent (that's what I get for looking at bugs after a full day of work). However worrying about the battery different between a SurfaceView and TextureView without data backing either assertion may be a little early; though I am interested in any data about that now :).

@ojw28
Copy link
Contributor

ojw28 commented Apr 15, 2016

  • We'll push a fix to correctly apply the scaling mode soon, which will be to set it on the MediaCodec from onOutputFormatChanged as described above.
  • TextureView is definitely significantly worse with respect to power draw. How much worse depends on many factors, but we've seen numbers in the region of a ~30% increase in overall device power draw when using TextureView rather than SurfaceView.

@brianwernick
Copy link

Thanks @ojw28.

@gbhall
Copy link

gbhall commented Apr 26, 2016

@ojw28 Any update? Thanks.

ojw28 added a commit that referenced this issue Apr 26, 2016
Issue: #1428
-------------
Created by MOE: https:/google/moe
MOE_MIGRATED_REVID=119952373
@ojw28 ojw28 closed this as completed Apr 26, 2016
@gbhall
Copy link

gbhall commented Apr 28, 2016

Cheers @ojw28. Got this working using the dev branch library project. Any idea when you'll be pushing an updated master release? P.S. the applied fix doesn't seem to work if you're obtaining a Surface from TextureView. Cheers.

@ojw28
Copy link
Contributor

ojw28 commented Apr 28, 2016

Scaling mode only works for SurfaceView. We'll clarify this in the documentation. If you're using TextureView then you should use TextureView.setTransform instead.

@gbhall
Copy link

gbhall commented Apr 28, 2016

@ojw28 Cheers man. I'll stick to SurfaceView because I think I read from you that it comes with up to a 30% performance boost. However the solution doesn't seem to work for videos recorded off phones (tested Nexus 5 and LG G3). I've opened up another issue to easier track this: #1482. Cheers.

ojw28 added a commit that referenced this issue Jun 15, 2016
Issue: #1428
-------------
Created by MOE: https:/google/moe
MOE_MIGRATED_REVID=119952373
@google google locked and limited conversation to collaborators Jun 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants