Add video autoplay to media gallery #4499

jmartinesp · 2025-03-28T13:48:20Z

Content

Add video autoplay to any video the user explicitly opens (either from the timeline or the media gallery). Scrolling between videos in the gallery shouldn't result in autoplaying any other videos.
Make sure reloading the media gallery items doesn't result in the video playback being paused then shortly resumed again.

Motivation and context

Fixes element-hq/element-meta#2797, achieving parity with iOS.

Tests

Open a video from the timeline, it should be displayed in the media gallery of the room you're in and autoplayed.
If you scroll to some other video, this one should not autoplay.
There are no accidental play/pause changes when new items are added to the media gallery.
Scrubbing the video, moving to a different timestamp, should keep working as expected.

Tested devices

Physical
Emulator
OS version(s): 14

Checklist

Changes have been tested on an Android device or Android emulator with API 24
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
Pull request is based on the develop branch
Pull request title will be used in the release note, it clearly define what will change for the user
Pull request includes screenshots or videos if containing UI changes
You've made a self review of your PR

github-actions · 2025-03-28T13:52:02Z

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/qAd8TN

codecov · 2025-03-28T13:58:28Z

Codecov Report

Attention: Patch coverage is 70.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 80.10%. Comparing base (c3ee00e) to head (4155990).
Report is 2 commits behind head on develop.

Files with missing lines	Patch %	Lines
...ies/mediaviewer/impl/local/video/MediaVideoView.kt	36.36%	7 Missing ⚠️
...braries/mediaviewer/impl/viewer/MediaViewerView.kt	60.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4499      +/-   ##
===========================================
- Coverage    80.11%   80.10%   -0.01%     
===========================================
  Files         2080     2080              
  Lines        55679    55701      +22     
  Branches      6812     6816       +4     
===========================================
+ Hits         44605    44620      +15     
- Misses        8737     8742       +5     
- Partials      2337     2339       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

… the media gallery

jmartinesp · 2025-03-31T10:52:53Z

Oops, there is some leftover code from experiments that should not be there anymore, sorry. I'll fix that ASAP.

jmartinesp · 2025-03-31T11:36:51Z

...in/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/MediaGalleryDataSource.kt

+        }
+        .distinctUntilChanged()
+        .map { timelineItems ->
+            val groupedItems = mediaItemsPostProcessor.process(mediaItems = timelineItems)
+            mediaTimeline.orCache(groupedItems)
+        }
+        .onEach { groupedMediaItems ->


No actual changes here, but Detekt complained about this so I made some changes to make it happy.

bmarty

This is working (thanks for the last fix).
However I see one point that could be improved:

opening a video;
auto play starts;
swipe to another media;
the video is automatically paused;
swipe back to the video.

The video auto start again but from the beginning, even if the user has manually paused it.

I believe that previously the media was paused and the player position was not lost, except if the user is swiping to much (in this case the View is disposed).

Maybe the video should not auto-play when swiping back to it?

jmartinesp · 2025-03-31T15:58:26Z

Maybe the video should not auto-play when swiping back to it?

I thought iOS was autoplaying when coming back, but it's not. So I think you're right, I changed this behaviour in my latest commit.

Copilot

Copilot wasn't able to review any files in this pull request.

Files not reviewed (11)

libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/MediaGalleryDataSource.kt: Language not supported
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/local/LocalMediaView.kt: Language not supported
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/local/audio/MediaAudioView.kt: Language not supported
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/local/player/MediaPlayerControllerState.kt: Language not supported
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/local/player/MediaPlayerControllerStateProvider.kt: Language not supported
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/local/video/MediaVideoView.kt: Language not supported
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSource.kt: Language not supported
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt: Language not supported
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerState.kt: Language not supported
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerStateProvider.kt: Language not supported
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerView.kt: Language not supported

bmarty

Thanks for the update!

I see that LocalMediaView is also used for the media attachment preview (see code in DefaultLocalMediaRenderer). But I think that it's not a good idea to auto-play in this case, so the current implementation is fine.

One last non-blocking question about resetting the player location when swiping between media.

bmarty · 2025-04-01T07:20:22Z

.../src/main/kotlin/io/element/android/libraries/mediaviewer/impl/local/video/MediaVideoView.kt

            exoPlayer.pause()
+            exoPlayer.seekTo(0)


Why resetting to player position here?

This was done so the video would restart from the beginning when coming back to it, but since we don't do that anymore I think we can remove it now. Thanks for noticing it!

…er one

sonarqubecloud · 2025-04-01T07:39:43Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Add video autoplay when opening the video from either the timeline or…

33eafa1

… the media gallery

jmartinesp force-pushed the fix/media-gallery-autoplay branch from 562c3ab to 33eafa1 Compare March 31, 2025 09:38

Fix lint issues

aa3ac45

jmartinesp added the PR-Bugfix label Mar 31, 2025

jmartinesp marked this pull request as ready for review March 31, 2025 10:46

jmartinesp requested a review from a team as a code owner March 31, 2025 10:46

jmartinesp requested review from bmarty and removed request for a team March 31, 2025 10:46

jmartinesp marked this pull request as draft March 31, 2025 10:52

Revert changes to MediaViewerDataSource during experiments

8d93877

jmartinesp commented Mar 31, 2025

View reviewed changes

jmartinesp marked this pull request as ready for review March 31, 2025 11:48

Fix check

96988e8

bmarty reviewed Mar 31, 2025

View reviewed changes

Only autoplay when the video is first displayed

3f9ffb7

manuroe requested a review from Copilot April 1, 2025 06:51

Copilot AI reviewed Apr 1, 2025

View reviewed changes

bmarty approved these changes Apr 1, 2025

View reviewed changes

Remove going back to the beginning of the video when swiping to anoth…

4155990

…er one

jmartinesp enabled auto-merge (squash) April 1, 2025 07:34

jmartinesp merged commit 3b35948 into develop Apr 1, 2025
29 checks passed

jmartinesp deleted the fix/media-gallery-autoplay branch April 1, 2025 07:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add video autoplay to media gallery #4499

Add video autoplay to media gallery #4499

jmartinesp commented Mar 28, 2025 •

edited

Loading

github-actions bot commented Mar 28, 2025 •

edited

Loading

codecov bot commented Mar 28, 2025 •

edited

Loading

jmartinesp commented Mar 31, 2025

jmartinesp Mar 31, 2025

bmarty left a comment

jmartinesp commented Mar 31, 2025

Copilot AI left a comment

bmarty left a comment

bmarty Apr 1, 2025

jmartinesp Apr 1, 2025

sonarqubecloud bot commented Apr 1, 2025

Add video autoplay to media gallery #4499

Add video autoplay to media gallery #4499

Conversation

jmartinesp commented Mar 28, 2025 • edited Loading

Content

Motivation and context

Tests

Tested devices

Checklist

github-actions bot commented Mar 28, 2025 • edited Loading

codecov bot commented Mar 28, 2025 • edited Loading

Codecov Report

jmartinesp commented Mar 31, 2025

jmartinesp Mar 31, 2025

Choose a reason for hiding this comment

bmarty left a comment

Choose a reason for hiding this comment

jmartinesp commented Mar 31, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

bmarty left a comment

Choose a reason for hiding this comment

bmarty Apr 1, 2025

Choose a reason for hiding this comment

jmartinesp Apr 1, 2025

Choose a reason for hiding this comment

sonarqubecloud bot commented Apr 1, 2025

Quality Gate passed

jmartinesp commented Mar 28, 2025 •

edited

Loading

github-actions bot commented Mar 28, 2025 •

edited

Loading

codecov bot commented Mar 28, 2025 •

edited

Loading