jellyfin-archive / jellyfin-android-original

Android Client for Jellyfin
https://jellyfin.org
GNU General Public License v2.0
271 stars 65 forks source link

Feature/native exo player #238

Closed Stampede10343 closed 4 years ago

Stampede10343 commented 4 years ago

Looking for feedback and help figuring out captions and different streams, like 720p/480p etc.

Stampede10343 commented 4 years ago

@vitorsemeano @dkanada This is my progress so far on the native player, the layout more closely matches the web player, still need some help figuring out the track selection stuff with captions and changing stream qualities.

vitorsemeano commented 4 years ago

You do have a lot of changes, thanks! I need to take some time tomorrow to review them and test them myself. After that i will give you my ideas to fetch the remaining info like available tracks and subtitles loading, as their selection.

vitorsemeano commented 4 years ago

You have some build errors that are easy to fix.

In plugin.xml, inside NativeShell plugin, add the resources ic_closed_caption.xml and ic_settings.xml to it, so compilation succeeds.

Also you need to add the android_theme for AppCompactActivity.

I see you made some changes to the screen always on when playing, a little advanced to me atm ahah.

The implemented approach that i made was commented out because is not the right one. When you click on the subtitles button, you could do in 2 different approaches:

If you want, i could do this in the following week.

Selecting video tracks could become more complex when direct streaming. When the video playing is not transcoding, the hls protocol is not used initially. So in comparison the web player allows this manual selection, but changes from a direct stream to a hls stream. This requires more discussion with the rest of the maintainers.

For now, subtitles is the next step, as any improvement to the UI is welcome, nice job!

Stampede10343 commented 4 years ago

Ahh, yeah I modified the plugin.xml a bit, but was having no issues locally, but figured those entries were in there for a reason. I'll get that updated as well as the android_theme stuff.

I'll take another crack at the subtitles soon, that makes sense but the whole track selector stuff makes sense but doesn't seem to work how I'd expect it to, and there's also some lack of knowledge on my end how the subtitles fit into a video when they aren't external. I've been trying to see how the other clients handle it but haven't gotten too far into finding that code yet.

Stampede10343 commented 4 years ago

I took another attempt at setting up subtitles, it seems like the track and renderer are getting enabled through the logs but I'm not seeing anything come through.. I can push what I have but its kind of a mess since I've been trying a bunch of things.

vitorsemeano commented 4 years ago

I can try to make subtitles work, and push to this PR. Would you allow me to push to it?

It will take a day or two so I have some time to look into your code and make the subtitles work (or trying xD).

Stampede10343 commented 4 years ago

Sounds good to me!

vitorsemeano commented 4 years ago

Hi, you didn't commit the styles.xml file, is giving a compilation error, can you commit it out? Thanks

Stampede10343 commented 4 years ago

Oh crap.. my bad I thought I got it in the last commit. Pushed.

vitorsemeano commented 4 years ago

Thanks! i will look into the subtitles issue

vitorsemeano commented 4 years ago

Hi, could you give me permissions to push to this PR?

vitorsemeano commented 4 years ago

Hi, i am still not authorized to push to this PR. Should i made a PR against your fork or should i wait for you to give me permissions to push to the branch Stampede10343:feature/native-exo-player in your fork?

Stampede10343 commented 4 years ago

Hmm it says you haven't responded to the collaborator invite, check your email.

vitorsemeano commented 4 years ago

I received the invitation, thanks.

I added basic subtitles support, now you can select some of the subtitles using the popupmenu that you implemented. Also external subtitles work out of the box. But it still needs some improvements like creating a checkbox or something similar indicating which subtitle is currently selected, also a option to remove the subtitle.

I also updated to the latest exoplayer version.

What's missing is selecting a subtitle that the player doesn't support, i believe dvdsub it can't handle it, needs to be transcoded in the server. Also hls streams don't bring the embed subtitles, so they need to be transcoded, i need to change the url stream so it includes the selected subtitle.

I also looked this morning how video/audio/subtitle track selection works in the html video player. All rely on the changeStream of the playbackmanager. So i need to create a method that talks with the jellyfin-web part and requests a stream change. Then i need to adapt the play method so if an instance is already running, it's only necessary to process the new uri (this will require pre loading of external subtitles i believe).

The video tracks will only select automatically depending on network in the presence of a hls stream (when transcoding). I need to look further to see how they force a transcode to a direct play stream.

You can make any improvements in my changes if you see they need such.

In the following day i could look into these track selections for audio and video. Tell me if in general subtitle selection is working as expected.

Also some subtitles render with a black background. I think you will need to extend a exoplayer view for subtitles and change the style of it to be transparent.

Stampede10343 commented 4 years ago

Looks good. I need to figure out the theme for cordova since the theme I had set isn't used, but I can address that shortly.

I think I had the right idea but the movies I was testing it on must have had unsupported subtitle streams since my code was pretty similar to what you have here. We need a way to figure out if there are even streamable subtitiles since it appears that most movies I checked will show an 'undefined' subtitle stream that does nothing, in those cases it would probably be best to grey out or hide the captions button altogether.

vitorsemeano commented 4 years ago

Hi, sorry for the late reply, i think i sorted it out the undefined option. It was related with subtitle device profile i was assigning. Also for tracks without label and language, i simple remove them from the popup menu.

Next thing i will look is the change stream option for subtitle if such subtitles can only be activated using transcoding. But this could take some days.

Stampede10343 commented 4 years ago

Sounds good! I need to take another look at fixing up the theming still. I'll check it out soon

vitorsemeano commented 4 years ago

Could you enable/implement the dropdowns for audio streams in the settings icon? even if there's no connection to exoplayer, so i can start from there to change those streams when the player requests it.

Take your time, there's no worries :)

Stampede10343 commented 4 years ago

Could you enable/implement the dropdowns for audio streams in the settings icon? even if there's no connection to exoplayer, so i can start from there to change those streams when the player requests it.

Take your time, there's no worries :)

Yup. I'm taking a look at it tonight, I'll probably have something in the next couple days

Stampede10343 commented 4 years ago

@vitorsemeano Just added some code to open another menu for selecting audio tracks, its just a shell but should let you drop in the tracks and selection logic and just work..

dkanada commented 4 years ago

@Stampede10343 @vitorsemeano how is progress for this pull request? The release I had planned is currently blocked by a potential performance issue, so I was just checking in.

Stampede10343 commented 4 years ago

@Stampede10343 @vitorsemeano how is progress for this pull request? The release I had planned is currently blocked by a potential performance issue, so I was just checking in.

It still needs a fair bit of work to get audio and video streams working. I need to take another look into it and will probably need more help or to dive into the web client for inspiration on the stream selection. Been meaning to get back to this.

ruCyberPoison commented 4 years ago

Great Work, @vitorsemeano @Stampede10343

We are waiting for the enhancement/feature :)

vitorsemeano commented 4 years ago

Hi, sorry for the late reply been a little way from here.

Managed to improve subtitle handling. I display the subtitle streams that come from jellyfin server, and then map the ones recognized by exoplayer with those streams, using the language associated with the track. Also if a stream can't be "direct played" in exoplayer, i call the playerManager requesting a stream change for that subtitle. That also implies that the player itselt is now ready for stream changes, not being necessary to reload the entire player. When that happens, the loading bar appears, and the playback stops. When the new streams kicks in, the playback is resumed with the new subtitle track.

We still need to add a option to remove a subtitle, i think is fairly simple, but needs some work. Audio tracks i need to catch them and map them the same way like subtitles, as also call playbackManager to change one when exoplayer can't do it (transcoding for example).

Tell me if the subtitle handling is better now.

Stampede10343 commented 4 years ago

Just did a quick review of the changes, looks like a great step in the right direction! I'll hopefully have time this weekend to take another look and do some more testing!

JustAMan commented 4 years ago

What makes this PR a draft for now?

vitorsemeano commented 4 years ago

Hi, yesterday I refactored the item that comes from web code and transform it into a java object, for code clarification and separation. Next step would be to make audio selection possible.

Video selection in pending changes on the server regarding hls stream as it should be. I don't think it justifies the creation of a bridge between the current implementation in the web part for video track selection and exoplayer. If in the future the server supplies a hls stream containing all video tracks as discussed in riot, that would be perfect, as exoplayer already support hls streams.

After the selection of streams is completed, then is only minor adjustments, like displaying metadata of the file being player, images, etc.

Stampede10343 commented 4 years ago

Yeah I'm mostly just using draft status as it's not feature complete yet and that unless you're super interested there's not much use reviewing it quite yet.

vitorsemeano commented 4 years ago

Hi, been some time. Today I bring another update to this implementation. I spent some hours refining the subtitle selection, as adding pretty neat things that was missing:

I have been testing with some files with multiple tracks for audio and subtitles, and so far is working as expected. I extracted all code related to buttons to another class, for simplicity and refactoring.

Also I detached all invocations made by the exoplayer UI to the web part using promises, avoiding dead locks when the web part takes too long.

The remaining part to this implementation is video track selection when hls is playing, and tracks handling associated with it. If the server provides a full hls master playlist with a bunch of tracks, should be enough for the exoplayer to handle it.

The objective is to allow a android client to adapt the streams depending on bandwidth limitations. Direct play should be automatically available depending on the available bandwidth between client and server.

I will try to discuss this matter with you guys and then proceed with this implementation.

@Stampede10343 when you have some time, you can test this version. I hope to receive some feedback from you soon :).

biaji commented 4 years ago

@Stampede10343 @vitorsemeano I would like to translate some strings into Chinese(simplified). Should I send the pr now or after these code merged to mainstream?

dkanada commented 4 years ago

@biaji I added this project to our Weblate instance, you you can easily translate strings there and they will show up in the next version.

biaji commented 4 years ago

@biaji I added this project to our Weblate instance, you you can easily translate strings there and they will show up in the next version.

Great~ I'll do it right now......

But I can't find the new string introduced by this feature branch in the weblate...

dkanada commented 4 years ago

It's only for strings on master, not this branch unfortunately. As soon as this gets merged you can add strings here though.

rexbron commented 4 years ago

Would this PR address issue #134? That's a really quality of life issue for using Jellyfin as a music server.

RT4U commented 4 years ago

This looks cool.

Any progress so far regarding this guys?

Considering the current world situation hope everyone are safe!

Stampede10343 commented 4 years ago

This looks cool.

Any progress so far regarding this guys?

Considering the current world situation hope everyone are safe!

All good here, just burnt out currently. The UI and major pieces are mostly in place but the audio/video tracks are really strange and any time I work with them it gets super discouraging because there's a lot of unknowns.

ylefebvre commented 4 years ago

As mentioned on another thread, someone's been working on their own Android audio client, called Gelli (https://github.com/dkanada/gelli). Perhaps some ideas could be brought in. It does seem to playback audio files quite well.

dkanada commented 4 years ago

I'm also a team member here :smile: Unfortunately almost nothing from my project would be of any benefit here though.

biaji commented 4 years ago

Maybe we can merge this feature branch right now and try to add/fix on the main stream?

dkanada commented 4 years ago

That sounds reasonable to me. If anyone wants to quickly implement the settings page we can also optionally enable this player as a beta within the application somehow.

Stampede10343 commented 4 years ago

Yeah @dkanada I think that sounds like a good plan. I'll hopefully come back around to this at some point.

biaji commented 4 years ago

Any update? I noticed that a new version has been released...

Stampede10343 commented 4 years ago

Any update? I noticed that a new version has been released...

I'm not really feeling like finishing this one up.. I don't really think the streaming stuff works correctly, or at least we see similar issues with the AndroidTV app having issues selecting streams.

dkanada commented 4 years ago

@Stampede10343 no worries, we were going to attempt a port of this pull request to the new client!

dkanada commented 4 years ago

ExoPlayer is working and enabled as a setting in the new client. I'll be archiving this repository and replacing it with the new one over the next week and we should have a release out sometime this weekend.