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

BUG: DebugViewProvider/AspectRatioFrameLayout is rotated 90 degrees. #1098

Open
steelbytes opened this issue Feb 16, 2024 · 11 comments
Open
Assignees

Comments

@steelbytes
Copy link

My input file is 4K landscape. For my output I want to crop left and right and scale to 1080x1920 portait for an Instagram Reel. So I add Presentation.createForWidthAndHeight(1080, 1920, Presentation.LAYOUT_SCALE_TO_FIT_WITH_CROP). Output file looks correct.

BUT DebugViewProvider/AspectRatioFrameLayout is rotated 90 degrees. I can't expect my users to rotate their phone while previewing.

media3 library 1.2.1.
pixel 7 pro with current android at time of writing.

(There's no way to demonstrate this via your demo app so I gave up trying to use the bug report form which is limited to the demo app hence wrote this as a feature request).

@marcbaechinger
Copy link
Contributor

marcbaechinger commented Feb 19, 2024

Can you please provide us with more detailed information what the problem is and how it can be reproduced? Please provide reproduction steps to make the issue actionable for us.

@andrewlewis
Copy link
Collaborator

If I understand correctly, this is about the debug preview in the transformer demo showing the video rotated through 90 degrees. Please let me know if that's not correct though.

As the name suggests this view is just for debugging not for end users. It shows the frames that are being passed to the video encoder when transcoding with Transformer, which are rotated when the output is portrait because the video is encoded in landscape then metadata is set on the output MP4 to rotate it to the requested orientation. So this is working as intended.

What is the use case for showing this to users? I wonder if using ExoPlayer with setVideoEffects would be a more suitable replacement for this.

@steelbytes
Copy link
Author

If I understand correctly, this is about the debug preview in the transformer demo showing the video rotated through 90 degrees.

correct.

As the name suggests this view is just for debugging not for end users. It shows the frames that are being passed to the video encoder when transcoding with Transformer, which are rotated when the output is portrait because the video is encoded in landscape then metadata is set on the output MP4 to rotate it to the requested orientation. So this is working as intended.

ok, you developed this view for debug but I don't see why it has to display wrong . If it's wrong and you're not concerned then why even make this view available in the library at all?

What is the use case for showing this to users?

my app has many controls for changing the transformation of the video. Being able to see the results of them without having to save the output or wait for it to finish and then having to open is important to the practical use of the controls.

I wonder if using ExoPlayer with setVideoEffects would be a more suitable replacement for this.

not familiar with ExoPlayer. all the docs I have read direct me to what I am doing, including your demo app

@andrewlewis
Copy link
Collaborator

The rotation gets applied at the muxer/container level, whereas frames are sent to the debug preview view (if set) upstream of that, at the end of the video frame processing pipeline.

We could add code to the way the debug preview renders to apply the same rotation. However, I think some users of the debug preview would prefer to see exactly the data that's sent to the encoder without this kind of postprocessing.

The reason to make this view available is to help developers see intermediate output without having to wait for the export operation to complete.

Please could you try using ExoPlayer.setVideoEffects? That lets you see changes to transformations as they are applied. You may want the feature mentioned on #1080 about supporting updating the current frame when the effect changes while the player is paused to be implemented.

@steelbytes
Copy link
Author

I'll stick researching ExoPlayer on my todo list

@steelbytes
Copy link
Author

Please could you try using ExoPlayer.setVideoEffects? That lets you see changes to transformations as they are applied. You may want the feature mentioned on #1080 about supporting updating the current frame when the effect changes while the player is paused to be implemented.

I tried ExoPlayer.setVideoEffects and in TextureOverlay.getTextureId(presentationTimeUs: Long) presentationTimeUs doesn't work. So sadly this won't help me.

@andrewlewis
Copy link
Collaborator

andrewlewis commented Mar 3, 2024

Please could you elaborate on "presentationTimeUs doesn't work"? I tried subclassing TextureOverlay and returning a new texture on each call with the current presentation time rendered into it. The presentation timestamp advanced as expected so I'm not sure what's broken here.

@steelbytes
Copy link
Author

steelbytes commented Mar 4, 2024

Please could you elaborate on "presentationTimeUs doesn't work"? I tried subclassing TextureOverlay and returning a new texture on each call with the current presentation time rendered into it. The presentation timestamp advanced as expected so I'm not sure what's broken here.

when using with Transformer the presentationTimeUs starts at 0. Good. when using with ExoPlayer it gives 1000000000000. WTF? Docs for ExoPlayer says "This feature does not work with effects updating the timestamps". Conclusion: I run away and work on something else as such an undcoumented and unexplained value won't be consistent across all devices

@andrewlewis
Copy link
Collaborator

when using with ExoPlayer it gives 1000000000000.

@claincly Can we subtract off the stream offset, like we do in processOutputBuffer for non-effects playbacks? That would make this consistent with Transformer, and the reason the offset is there is just to avoid negative timestamps going to the decoder (and we are downstream of the decoder in effects playback).

This feature does not work with effects updating the timestamps

Do you actually need to modify video frame timestamps in your custom effect(s)? That isn't supported yet and is what this note is about (but we don't expect it to be a significant limitation for most apps because shifting frame timestamps can cause audio and video to be desynchronized).

@claincly
Copy link
Contributor

claincly commented Mar 5, 2024

I can try doing that - will need to experiment with MediaItem change in a playlist as the stream offset is likely to change. Will investigate further.

@steelbytes
Copy link
Author

Do you actually need to modify video frame timestamps in your custom effect(s)? That isn't supported yet and is what this note is about (but we don't expect it to be a significant limitation for most apps because shifting frame timestamps can cause audio and video to be desynchronized).

No. I am simply changing the content of a bitmap every frame based on the frame timestamp combined with some other data. (and of course transferring it to the gl texture)

It's actually telemetry from my vespa and gps data overlaid on a gopro video. example output https://www.youtube.com/shorts/TQi86PkIlPE

copybara-service bot pushed a commit that referenced this issue Jun 25, 2024
This is consistent with `Transformer` and `CompositionPlayer`

Issue: #1098
PiperOrigin-RevId: 646446824
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants