teras / Jubler

Jubler Subtitle Εditor
http://www.jubler.org/
GNU General Public License v2.0
145 stars 17 forks source link

Formatted ASS subtitle makes it impossible to read in mplayer preview #37

Open 1dolla opened 5 months ago

1dolla commented 5 months ago

(macOS, Jubler 8.0.0, mplayer 1.5-4.2.1 via MacPorts)

Maybe I'm completely missing something here, but I've been trying to set up mplayer properly with Jubler, so I can have some nice previews of what I'm editing.

I'm dealing with 4k and 8k videos, so I use -xy 800 as mplayer argument to avoid the mplayer window taking up the entire screen and blocking the Jubler window.

The subtitles are then absolutely tiny and unreadable. So I try to add arguments to increase the size of the subtitle in the mplayer window, but because the temporary subtitle file that Jubler creates is an ASS file with specific font directives, e.g.:

/var/folders/k5/jb4pjqn14ms8ph8kxb71q2v00000gn/T/jubler_11295766013046402661.ass

[V4+ Styles]
Format: Name, Fontname, Fontsize, PrimaryColour, SecondaryColour, OutlineColour, BackColour, Bold, Italic, Underline, StrikeOut, ScaleX, ScaleY, Spacing, Angle, BorderStyle, Outline, Shadow, Alignment, MarginL, MarginR, MarginV, Encoding
Style: Default,American Typewriter,14,&H00FFFFFF,&H0000FFFF,&H4B000000,&H4B404040,0,0,0,0,100,100,0,0,1,0,2,2,20,20,20,0

then mplayer ignores font rendering arguments like -subfont-text-scale.

Changing the font size to the max available 72 makes the subs just legible in mplayer for 4k videos, still unreadable for 8k videos. And with the side effect that the Jubler UI is now unusable because the subs are huge.

How do I resolve this? Am I simply blind that I can't find the option to use fractional sizes, or to not use ASS as a temporary format, or to not format the ASS as that's not a requirement, but an option for that format? The sub is loaded and saved as SRT. I've been trying things for hours.

Help :'(

1dolla commented 5 months ago

Okay, so I hacked around since I wrote that, and made this patch:

diff --git a/modules/core/src/main/java/com/panayotis/jubler/media/player/AbstractExternalPlayer.java b/modules/core/src/main/java/com/panayotis/jubler/media/player/AbstractExternalPlayer.java
index 0d261444..b4fb985c 100644
--- a/modules/core/src/main/java/com/panayotis/jubler/media/player/AbstractExternalPlayer.java
+++ b/modules/core/src/main/java/com/panayotis/jubler/media/player/AbstractExternalPlayer.java
@@ -24,6 +24,10 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.StringTokenizer;

+import com.panayotis.jubler.media.VideoFile;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 public abstract class AbstractExternalPlayer extends ExternalVideoPlayer implements PluginItem<AvailExternals> {

     private AbstractPlayerOptions opts;
@@ -90,6 +94,18 @@ public abstract class AbstractExternalPlayer extends ExternalVideoPlayer impleme
         options = options.replaceAll("%\\(", "");
         options = options.replaceAll("%\\)", "");

+        /* is mplayer launched with a specific width in pixels? if so, update videoplayer width and height values to match, so that the font size in .ass renders as intended: */
+        Pattern pattern = Pattern.compile("-xy ([0-9]+)");
+        Matcher matcher = pattern.matcher(options);
+        if (matcher.find())
+        {
+            VideoFile vfile = mfile.getVideoFile();
+            int newWidth = Integer.parseInt(matcher.group(1));
+            double heightFactor = (double) vfile.getHeight() / (double) vfile.getWidth();
+            int newHeight = (int) Math.round(newWidth * heightFactor);
+            vfile.setInformation(newWidth, newHeight, vfile.getLength(), vfile.getFPS());
+        }
+
         /* tokenize command line */
         StringTokenizer st = new StringTokenizer(options, " ");
         String[] cmds = new String[st.countTokens()];

I haven't programmed in some years, and never in Java, and "bom," "maven," and all that is pretty much mumbo jumbo to me on top. I had a hard time figuring out where in your code to fiddle to achieve what I wanted. It looks nice, though, AFAICT from my limited knowledge of Java and mplayer et al.

Basically what I've come to understand this noon is that the dimensions specified in PlayResX/PlayResY in an ASS file are to indicate the width and height in pixels of the viewport in which the author of the subtitle viewed the content. The font size is then in pixels mapped onto that viewport. So that if the viewer's viewport is, say, 50% larger than the author's, the player should scale the fonts by +50% as well, and vice versa.

In that respect, what I've done here — if still somewhat hacky and clumsy — is the correct way to go. It should, of course, just be an option in Jubler to launch mplayer at a specific width, and then use THAT value for specifying PlayResX etc. Without controlling the size of the mplayer window, you can't really know the correct values to put there. If I launch an 8k video on a 2k screen, mplayer will just reduce the size, and then the ASS viewport values would be off by a factor of four.

I don't know, it solves my issue, and I thought I'd share my thoughts.

BTW, this could be purely because of my ignorance, but I was unable to build the entire app, because appenh was trying to download a appenh-0.7.pom file that doesn't exist as a dependency. I don't understand maven or pom or jars really, but it sounds like it's saying it's dependent on itself? Anyway, I just built core.jar instead and replaced the one in Jubler.app with it.

teras commented 5 months ago

First of all thank you for the patch. I appreciate it. Let me perform a few tests first and I'll integrate it in some sort soon.

1dolla commented 5 months ago

Cool, I'm glad it makes sense :) Thank you.

teras commented 5 months ago

As a first review, I am thinking of moving this code to the mplayer specific code, instead of the generic external player.

teras commented 5 months ago

BTW, this could be purely because of my ignorance, but I was unable to build the entire app, because appenh was trying to download a appenh-0.7.pom file that doesn't exist as a dependency. I don't understand maven or pom or jars really, but it sounds like it's saying it's dependent on itself? Anyway, I just built core.jar instead and replaced the one in Jubler.app with it.

This was my bad, I am sorry.

The appenh 0.7 is now live

Just to make things clear for people not familiar with this; it's not trivial to have a dependency like this live. You need to upload to the world to make it available for everyone.