moneytoo / Player

▶ Simple and lightweight, yet polished and powerful Android video player based on ExoPlayer
https://play.google.com/store/apps/details?id=com.brouken.player
The Unlicense
1.76k stars 188 forks source link

Incorrect subtitles encoding for streamable URL #586

Open JJD2K opened 5 months ago

JJD2K commented 5 months ago

Describe the bug I am trying to use Just as external player from NOVA to play files over SMB. Unfortunately NOVA launches external players in a strange way and they do not have access to subtitles. That is why NOVA passes the subtitles as streamable URL, but in that case Just does not detect properly the encoding of the files and displays ??????????? (question marks in rhomboids). Please note that the same subtitles are displayed properly if I launch Just from MIxplorer or other file manager, because they probably set working directory for JUST and just loads the subtitles in a different way. This happens on files with Windows-1251 encoding (probably others) too.

See this link: https://github.com/nova-video-player/aos-AVP/issues/1177

Could you please check this issue? Thank you in advance.

To Reproduce

  1. Prepare a movie that has external subtitles file in Windows-1251 encoding and store the files on an SMB share.
  2. Name the subs wit the name of the movie.
  3. Set Just as an external player in NOVA and open the video from NOVA
  4. The subtitles text is displayed as question marks in rhomboids on the screen

Expected behavior The encoding should be detected and used the same way Just does when loading a file from Mixplorer or other file manager.

Device (please complete the following information): Android TC

Media Any media with Windows-1251 encoded external .sub file

Additional context Add any other context about the problem here.

moneytoo commented 5 months ago

Subtitle charset detection and conversion is not done for subtitles passed to Just Player using the MX Player like API. My reasoning was:

JJD2K commented 5 months ago

@moneytoo Hey thank you for getting back to me. I totally agree with you, but the developer of Nova does not care to fix those issues. I do not know why such seemingly simple things are so hard for him. I do not understand why he keeps adding features that no one uses like opensubtitles (most subtitles there are auto-translated), while core functionality like fluent playback, proper audio passthrough and subtitles are not working.

I agree that UTF-8 should be used, but there are billions of files in other encodings and that burden will not go away any soon.

Your player can already load Windows-1251 encoded subs from a file properly. It should not be that hard for you to apply those to the sreamable URL too. About breaking encoding - you already have preferred language in settings that you can use as a reference. Plus you already load subs from file with proper encoding.

Could you please add a fix for this as the developer of NOVA can not handle it? I proposed him multiple solutions, but I guess it is too hard for him. I saw his subs code and it is complete mess. I am not an Android developer and it will cost me a lot of time to download NOVA and setup the build environment to fix something that can be fixed in 30 minutes, but probably at the end I will download and build my own fixed version and use it.

courville commented 2 months ago

@JJD2K, I do care about fixing issues. However, when passing subs to an external player, I just do not think that it is nova's responsibility to do the transcoding for somehow the same reasons @moneytoo provided.

@moneytoo, if there is something better that nova should do please let me know. For now nova acts as an http proxy to pass http URIs to external player for the video and all subs (this way it can cope with network shares "transparently" cf. PlayUtils).

EDIT: I tried to download the subs and pass them as files to just player but for some reasons it does not work

+++ b/src/main/java/com/archos/mediacenter/video/utils/PlayUtils.java
@@ -361,12 +361,25 @@ public class PlayUtils implements IndexHelper.Listener {
                     } else {
                         // TODO FIXME passing file to 3rd party player is not working
                         subFile = new File(subPath);
-                        subUri = FileProvider.getUriForFile(context, "org.courville.nova.provider", subFile);
+                        subUri = FileProvider.getUriForFile(context, BuildConfig.APPLICATION_ID + ".provider", subFile);
                         MxSubPaths.add(subUri);
                     }
+                    /*
+                    // TEST
+                    File file = new File(filePath);
+                    Context context = getContext(); // or any method to get context
+                    String authority = context.getApplicationContext().getPackageName() + ".provider";
+                    Uri contentUri = FileProvider.getUriForFile(context, authority, file);
+
+                    Intent viewIntent = new Intent(Intent.ACTION_VIEW);
+                    viewIntent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION); // Grant temporary read permission
+                    viewIntent.setDataAndType(contentUri, context.getContentResolver().getType(contentUri));
+                    viewIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); // if you call from outside of an Activity context
+                     */
                     log.debug("onResumeReady: subPath " + subPath + " -> subUri " + subUri + "-> subLanguage " + subLanguage);
                     n++;
                 }
+                log.debug("onResumeReady: external cache dir " + mContext.getExternalCacheDir());
                 Parcelable[] MxSubPathsParcelableArray = MxSubPaths.toArray(new Parcelable[0]);
                 log.trace("onResumeReady: subs passed to 3rd party player " + Arrays.toString(MxSubPathsParcelableArray));
                 if (! MxSubPaths.isEmpty()) {
JJD2K commented 2 months ago

@courville , @moneytoo I don't understand why implementing such basic core features is such a big issue. Both applications already have code to detect encoding and this should be very simple to reuse.

@moneytoo

"Basically all texts across Android already are in UTF-8"

It would be nice if that was true, but non-utf-8 subtitles will not go away any soon - there are millions of files created over the years and people still keep using different encodings. Those should not be neglected.

"Sending application has potentionally more context to know whatever the subtitle is or isn't UTF-8 encoded" Whether you load the file or receive it as a stream does not make any difference IMHO.

"Conversion to UTF-8 costs some I/O plus storage of temporary subtitles in cache" You already do that when loading files - should not matter for a stream.

"And more importantly the charset detection can theoretically break properly encoded UTF-8 subtitles as detection is not 100%" Same as above - you already do that for files.

I have not thought about the fact that NOVA might be sending multiple subtitles for the same video. In such case it is probably more convenient JUST player to detect and apply the proper encoding only to the selected (loaded) file. If there are 20 subs for a file it is useless to process them all if they will not be used.

There are millions of subtitles and millions of users have non-utf files and I hope you both can work out a way to fix this as it is a CORE functionality. As I wrote - both apps can already load non-utf files so the code should be already there.

Thank you.