Migrate to media3

[parneet-guraya]

Solves: #4157

What this PR includes?

• Migration to media3 from Exoplayer
• Migration from PreviewVideoFragment to PreviewVideoActivity

Activity Migration Working Demo

Record_2023-10-03-19-44-46.mp4

Media3 migration demo

Record_2023-10-03-19-45-20.mp4

[parneet-guraya]

Hey @JuancaG05 👋 , I pushed the PR, have a look and let me know if it needs any changes. 🚀

[JuancaG05]

This is SOOOOOOO COOL @parneet-guraya! Great job! I like it a lot! 👏
Just left some comments and suggestions here for you to review before we move it to QA 😃

[parneet-guraya]

Hi, I resolved the changes and pushed the commits you asked and commented to some of the queries you had. Have a look.

[JuancaG05]

Nice @parneet-guraya! Approved from my side 💯! Thanks for applying the changes I requested and answering my questions 😃
Let's move this on to QA and let it in @jesmrec's hands 🎉

[jesmrec]

hi @parneet-guraya. Now, it's time for the QA after the code review was approved. It's a functional review in order to find defects or bad behaviours that hurts the user experience. In case i find any of them, i'll report in separated messages with all the information required to reproduce. Let's go for it!

[jesmrec]

QA checks:

• Video in different orientations
• Full screen
• Video controls -> right button (right arrow... always greyed out)
• Share option
• Remove option locally (if downloaded)
• Remove option remotely (if not downloaded)
• Open with option
• Send option (downloaded)
• Sync option (if downloaded) (check if useful, does nothing)
• Details option Migrate to media3 #4177 (comment)

[jesmrec]

(1)

1. Play a video
2. Click on 3-dot-button (top right corner) and then, Details

Current: Top bar shows ownCloud and back arrow to navigate is missing. After some seconds, file starts to download.
Expected: Details view is correct. File should not download.

Not sure how the video preview and the details view are connected and exchange info, but this behaviour is not reproducible in master but in the current branch.

Pixel2 Android11
Samsung A8 Android13

[jesmrec]

It took my attention that the right play-next is always greyed out. The left one moves to the initial point, so i guessed that it moves to the end or to the next video, but it does not.

I tried to google a little bit but i found nothing interesting, any further idea?

[parneet-guraya]

(1)

1. Play a video
2. Click on 3-dot-button (top right corner) and then, Details

Current: Top bar shows ownCloud and back arrow to navigate is missing. After some seconds, file starts to download. Expected: Details view is correct. File should not download.

Not sure how the video preview and the details view are connected and exchange info, but this behaviour is not reproducible in master but in the current branch.

Pixel2 Android11 Samsung A8 Android13

Hi @jesmrec, yes you're right it does do what you've said. Actually earlier we were using fragment and it was in the main activity so handling details fragment was being done from there. Since, we moved to the activity(from fragment) I need to launch the details fragment separately somehow. I am looking into it.

[parneet-guraya]

It took my attention that the right play-next is always greyed out. The left one moves to the initial point, so i guessed that it moves to the end or to the next video, but it does not.

I tried to google a little bit but i found nothing interesting, any further idea?

So, here's how these buttons supposed to work -->

• the previous button will take you to the previous media in the list of media items (but first it will take us to the begining of current media if it beyond the starting point.)
• the next button is also for taking us to the next media item in the list.

Since, we're only rendering single item (not passing a list of media items) they are not much of a use. You can see how google drive's doing it. It removes both next and previous buttons from the view. We can also do that , see below 👇

[jesmrec]

Since, we're only rendering single item (not passing a list of media items) they are not much of a use. You can see how google drive's doing it. It removes both next and previous buttons from the view. We can also do that , see below

I'd do that because it's confusing for the user: one item disabled and the other with a non-clear meaning. I'd keep the other three. Feasible then @parneet-guraya ?

[parneet-guraya]

Since, we're only rendering single item (not passing a list of media items) they are not much of a use. You can see how google drive's doing it. It removes both next and previous buttons from the view. We can also do that , see below

I'd do that because it's confusing for the user: one item disabled and the other with a non-clear meaning. I'd keep the other three. Feasible then @parneet-guraya ?

Yup 👍 working on it.

[parneet-guraya]

(1)

Current: Top bar shows ownCloud and back arrow to navigate is missing. After some seconds, file starts to download. Expected: Details view is correct. File should not download.

Hi, @jesmrec @JuancaG05 , regarding this behaviour I found out that this is also happening in the image preview (current master). This behaviour is being reproduced in master branch 👇

Record_2023-10-11-19-56-15.mp4

Both image preview and now video preview are activities and launching details fragment from activity has broken logic. So do you think I can may be create another issue for this and push the PR for that (solving bug for both of these activities) ?

Also, I removed the next and previous buttons. Have a look 👇

Record_2023-10-11-20-12-15.mp4

[jesmrec]

Hi, @jesmrec @JuancaG05 , regarding this behaviour I found out that this is also happening in the image preview (current master). This behaviour is being reproduced in master branch 👇

You're right. In master the behaviour is reproducible with images, but with videos. Are you reproducing in master with videos? In any case, we could address this specific problem to another issue. But, the problem of the download (after opening Details view, the streamed video starts to download) should be fixed. It's not reproducible in master so we`d be introducing a regression.

Also, I removed the next and previous buttons. Have a look

That's good!!!

[JuancaG05]

You're right. In master the behaviour is reproducible with images, but with videos. Are you reproducing in master with videos? In any case, we could address this specific problem to another issue. But, the problem of the download (after opening Details view, the streamed video starts to download) should be fixed. It's not reproducible in master so we`d be introducing a regression.

In master we cannot reproduce that with videos since they are previewed in a fragment, it was in this PR that @parneet-guraya moved video preview from a fragment to an activity, that's why it is now reproducible.

Do you think you can upload a quick fix/patch for the download issue so that we don't introduce new bugs and then move the details fragment wrong triggering to another issue?

[parneet-guraya]

In master we cannot reproduce that with videos since they are previewed in a fragment, it was in this PR that @parneet-guraya moved video preview from a fragment to an activity, that's why it is now reproducible.

Hi, @JuancaG05 thanks for the explaination to @jesmrec.

Do you think you can upload a quick fix/patch for the download issue so that we don't introduce new bugs and then move the details fragment wrong triggering to another issue?

Yes, I did push the hotfix have a look 👇

Record_2023-10-15-15-44-51.mp4

Also, I found another Issue also related to activity. See below 👇

Record_2023-10-15-16-00-23.mp4

This is what is happening -->

• We open the image preview.
• Go to details page.
• then press back twice and we are at the home screen (root folder).
• upon pressing back it again shows the image preview (instead of exiting the app which is expected).
• Then we go to the root screen and then it finally exits the app.

It is also reproducable in the master just like the other issue (appbar issue) and now this issue also present in video preview (since video preview is in activity).

Here's what I think the reason is (from a quick look at the behaviour) -->

• we launch the image preview.

• then we go to details screen for this we start the FileDisplayActivity again to show details fragment.

• So at this point we have this in our back stack (read from bottom to up from the list)
  - FileDisplayActivity (when we launch this activity for showing details fragment)
  - PreviewImageActivity (when launch the image preview)
  - FileDisplayActivity (when we launched the app for the first time)

• Here when we press back this is what happens -->
  - details fragment destroys on the first click and upon second click we go to back in directory and it is root. Now, when
  we press back again logic calls finish() hence this FileDisplayActivity destroys (not exits the app because not the last item in the backstack)
  - Now we can see PreviewImageActivity since this was the activity below. So, upon back press it exits the PreviewImageActivity.
  - now we can again see the images directory and upon clicking back we go up in the directory and it is root. Then when we click back it calls finish() and this time FileDisplayActivity is the last item in the back stack hence app exits.

Like the appbar issue this should also be raised in separate issue.

[JuancaG05]

Yes, I did push the hotfix have a look 👇

You rock!! 🎸

• So at this point we have this in our back stack (read from bottom to up from the list)
  • FileDisplayActivity (when we launch this activity for showing details fragment)
  • PreviewImageActivity (when launch the image preview)
  • FileDisplayActivity (when we launched the app for the first time)
• Here when we press back this is what happens -->
  • details fragment destroys on the first click and upon second click we go to back in directory and it is root. Now, when
    we press back again logic calls finish() hence this FileDisplayActivity destroys (not exits the app because not the last item in the backstack)
  • Now we can see PreviewImageActivity since this was the activity below. So, upon back press it exits the PreviewImageActivity.
  • now we can again see the images directory and upon clicking back we go up in the directory and it is root. Then when we click back it calls finish() and this time FileDisplayActivity is the last item in the back stack hence app exits.

Yes, it seems the problem is this very clearly. The solution could be just removing the previous stack somehow when we are in a preview screen (at least when it is an activity) and we go to the details screen. But as you said, this can be addressed in another issue, maybe all together with the other bug in the details screen. I will create the issue 👍

[jesmrec]

Thanks @parneet-guraya for fixing the problem, and sure, thanks for your engagement.

Thanks @JuancaG05 for opening new issue with the remaining problems with the browsing.

Work is done from my side. This one can be moved forward to master

[JuancaG05]

Here is the new issue I created with the existing problems in the details screen: #4188

[parneet-guraya]

Thanks @parneet-guraya for fixing the problem, and sure, thanks for your engagement.

Thanks @JuancaG05 for opening new issue with the remaining problems with the browsing.

Work is done from my side. This one can be moved forward to master

Happy to contribute as always 😄

[JuancaG05]

One last thing @parneet-guraya, there's a KtLint report that prevents BitRise from passing:
owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt:450:63 Missing spacing after ","

It's pretty simple, could you take a last look? 🤓

[parneet-guraya]

One last thing @parneet-guraya, there's a KtLint report that prevents BitRise from passing: owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt:450:63 Missing spacing after ","

It's pretty simple, could you take a last look? 🤓

done.