feat: Use online player for offline playback#7928
feat: Use online player for offline playback#7928xlash123 wants to merge 5 commits intolibre-tube:masterfrom
Conversation
|
Problems and Errors:
Also, if there is no internet, you may need to gray out some buttons to avoid errors. XRecorder_20251118_01.mp4Crash log:
XRecorder_20251118_10.mp4
XRecorder_20251118_04.mp4
XRecorder_20251118_06.mp4
XRecorder_20251118_11.mp4 |
|
I believe I have fixed those problems. It should prefer to use the metadata from the online The I also found that offline shuffle was crashing and fixed that too. |
XRecorder_20251118_13.mp4
XRecorder_20251118_16.mp4XRecorder_20251118_31.mp4
XRecorder_20251118_20.mp4
XRecorder_20251118_22.mp4
XRecorder_20251118_24.mp4
XRecorder_20251118_29.mp4
XRecorder_20251118_33.mp4
XRecorder_20251118_34.mp4
|
Bnyro
left a comment
There was a problem hiding this comment.
Hey, thanks for the PR! I think this is a definitely a good idea user experience-wise.
The way I did this was by merging the functionality of the OfflinePlayerService into the OnlinePlayerService
I see why you did that, but that will make it very complicated to maintain that code in the future.
First, the StreamItem type is not the one perfect fit here, we should instead use the Streams type because it already contains that information so you don't have to edit StreamItem.kt. The information in StreamItem.kt is only meant for video objects from search results or similar places, which don't contain any information like license, metaInfo, ...
The better approach would be the following:
- Create some more abstract methods in
AbstractPlayerServicethat work with theStreamstype - Implement them in both the
OnlinePlayerServiceand theOfflinePlayerService - Make
PlayerFragment.ktwork with theAbstractPlayerServiceclass instead of theOnlinePlayerServiceclass
Let me know if you have some questions about how that would work, don't hesitate to ask :)
|
I appreciate the feedback; always good to know the right way to implement this to enable good maintainability. I went into this just trying to get it working so I could show off the vision and get insight on how to better implement this. I like the idea of implementing through the |
I have tested again and would you please take a look at the above? |
Yes, exactly. We could also create a different data class instead of using |
|
@xlash123 Is this PR still active? |
|
Yes, I just haven't had much free time recently to devote to this unfortunately. |
|
Inspired by your PR, I've started a completely new implementation of your idea with #8206. Help with testing would be very appreciated :) Because of that, I'm closing your PR now. Thanks for your contribution! |
This patch allows for using the standard online player
PlayerFragmentfor both online and offline video playback. This will replace the need for theOfflinePlayerActivityand allow for a single method of video playback.The way I did this was by merging the functionality of the
OfflinePlayerServiceinto theOnlinePlayerService. I also used the metadata from theStreamItemin thePlayerFragmentto act as a the common interface for accessing media metadata, sinceDownloadalready supported a.toStreamItem()function.I've tested the following related parts to ensure working functionality:
PlayOfflineDialogstill works for selecting which version of the video to playPlayOfflineDialogThis is my first time really getting into Android development (and first time using Kotlin), so if you'd like me to make changes to clean this up, I am happy to do so and am open to suggestions on this. I hope you find this useful!