Video preview: use fullscreen dialog for video instead of new activity

[AlvaroBrey]

This allows transfering the playback directly between Player views, thus avoiding creating
a new ExoPlayer, re-loading the stream, having to pass playing status/current position, etc.

In summary, this provides a much more seamless experience when switching to/from fullscreen.

Additionally:

• Always enable buffering animation so it's clear when a video is loading
• Add padding to fullscreen duration numbers so they don't get cut off on rounded screens

• Tests written, or not not needed

[codecov]

Codecov Report

Merging #11089 (4fc73f0) into master (f9e500b) will increase coverage by 0.55%.
The diff coverage is 0.00%.

❗ Current head 4fc73f0 differs from pull request most recent head 3f990c3. Consider uploading reports for the commit 3f990c3 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #11089      +/-   ##
============================================
+ Coverage     30.87%   31.43%   +0.55%     
- Complexity     3280     3320      +40     
============================================
  Files           564      564              
  Lines         41616    41618       +2     
  Branches       5657     5638      -19     
============================================
+ Hits          12849    13082     +233     
+ Misses        26833    26547     -286     
- Partials       1934     1989      +55     

Impacted Files	Coverage Δ
...java/com/nextcloud/client/di/ComponentsModule.java	0.00% <ø> (ø)
...cloud/android/ui/activity/FileDisplayActivity.java	27.08% <0.00%> (-0.09%)	⬇️
...cloud/android/ui/preview/PreviewMediaFragment.java	0.00% <0.00%> (ø)
...android/ui/preview/PreviewVideoFullscreenDialog.kt	0.00% <0.00%> (ø)
.../com/owncloud/android/ui/dialog/LoadingDialog.java	0.00% <0.00%> (-95.24%)	⬇️
...com/owncloud/android/ui/activity/FileActivity.java	22.64% <0.00%> (-2.97%)	⬇️
...cloud/android/ui/helpers/FileOperationsHelper.java	5.87% <0.00%> (-2.28%)	⬇️
...wncloud/android/providers/FileContentProvider.java	72.22% <0.00%> (-1.86%)	⬇️
...a/com/owncloud/android/utils/FileStorageUtils.java	55.88% <0.00%> (-1.84%)	⬇️
...ud/android/providers/DocumentsStorageProvider.java	52.13% <0.00%> (-0.54%)	⬇️
... and 23 more

[tobiasKaminsky]

That is a super idea!
I tested it with two videos

• one from $internet: looks good
• one captured on same emulator: ratio is totally off

[AlvaroBrey]

• one captured on same emulator: ratio is totally off

This looks like a pre-existing bug that just shows here easier. The video that shows the problem is stored in horizontal but has a rotation metadata. There was a bug in Android with this, but it was patched in Android 11 (API 30). Reference: google/ExoPlayer#1482 There are other bugs with SurfaceView and metadata rotation, and workarounds with TextureView.

In the master branch and previous releases, this is reproducible by:

• Going to fullscreen and then back
• Opening preview fragment, leaving the app by Home and then coming back from Recents. Video (stable-3.22):
  rotation.webm

It looks like the bug is only triggered when the video is "resumed" on a surface, either a new one or the same one it was already on.

I can reproduce this in api 29 and lower, but can't reproduce in a higher API level. I can also only reproduce it with the 29 - Google Play image, not with the 29 - google APIs one. Fix may have been backported?

There may be a possible (but fairly complicated) workaround by using TextureView and applying the rotation manually. This would have to be confined to android <=29, as TextureView is more battery-consuming than the default SurfaceView.

@tobiasKaminsky what do you think we should do here? Attempt the workaround for API <29, or just let it be? It only happens with some <=29 devices, and only with videos with metadata rotation.

[tobiasKaminsky]

First, great that you figured it out!

@tobiasKaminsky what do you think we should do here? Attempt the workaround for API <29, or just let it be? It only happens with some <=29 devices, and only with videos with metadata rotation.

Actually I have no idea either. <=29 is quite a lot of users, which might be not a problem, depending how many videos actually use this rotation info.

There may be a possible (but fairly complicated) workaround by using TextureView and applying the rotation manually. This would have to be confined to android <=29, as TextureView is more battery-consuming than the default SurfaceView.

How long would this roughly take?

Then we could first merge without above workaround, but when to many users suffer by it, we can use this workaround or revert.

Another option would be to have this only for >29 enabled, and once we drop 28 support, we can change it.

[AlvaroBrey]

Another workaround idea: in devices with api <= 29, keep using the old approach (new exoplayer rather than transferring the old one). This will invalidate the UX gains for api 29 and under, though.

[AlvaroBrey]

How long would this roughly take?

I have no idea :( I think the workaround in my previous comment may be preferrable

[AlvaroBrey]

Rotation bug worked around in 4fc73f0

[AlvaroBrey]

/rebase

[github-actions]

Codacy

Lint

Type	master	PR
Warnings	82	82
Errors	0	0

SpotBugs

Category	Base	New
Bad practice	27	27
Correctness	45	45
Dodgy code	336	336
Internationalization	9	9
Multithreaded correctness	9	9
Performance	58	58
Security	18	18
Total	502	502

[github-actions]

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/11089.apk



To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

[tobiasKaminsky]

Works perfectly fine with "broken" video.