waddou / libass

Automatically exported from code.google.com/p/libass
1 stars 0 forks source link

Subtitles: Special Characters Break Custom Formatting #104

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. View soft-subbed video file with affected characters, e.g. 
https://dl.dropboxusercontent.com/u/3219541/OP.mkv
2. Watch up to time(s) of affected subtitle line(s), e.g. 21s, 1m16s & 1m28s

What is the expected output? What do you see instead?
I expect to see what MPC-HC gives (using whatever it uses), i.e. 
https://dl.dropboxusercontent.com/u/3219541/mpc_subtitles.jpg
Instead any characters /after/ the affected ones lose their custom formatting 
(fonts etc.), i.e. https://dl.dropboxusercontent.com/u/3219541/vlc_subtitles.png

What version of the product are you using? On what operating system?
I think it's libass 0.10.1. I get this with VLC versions 2.0.6, 2.0.7, and the 
current 2.1.0 nightly. I'm doing this in Windows (8), but it'd be great if 
someone could test this in Linux/Mac.

Please provide any additional information below. If relevant, please attach
the ASS/SSA script file and any fonts used in it.
I don't seem to have access to the attachment feature.

If there's further information needed to approach this problem, just let me 
know. :)

Original issue reported on code.google.com by suiseis...@gmail.com on 15 Jun 2013 at 7:20

GoogleCodeExporter commented 8 years ago
I had submitted this to the VLC Trac at 
https://trac.videolan.org/vlc/ticket/8799 but in the (linked) forum thread and 
IRC they suggested I submit something here too.

Original comment by suiseis...@gmail.com on 15 Jun 2013 at 7:22

GoogleCodeExporter commented 8 years ago
This is indeed a libass problem, although technically this is caused by errors 
in the subtitles in that file. They do {\rs} without defining a style "s". 
libass resets to the Default style, but it should instead reset to the current 
line's initial style, as if the tag was a simple {\r}.

Original comment by chortos@inbox.lv on 15 Jun 2013 at 8:06

GoogleCodeExporter commented 8 years ago
I've pushed a fix to my fork (master branch). While I was at it, I made another 
commit with a few VSFilter compatibility fixes for style name parsing.

Original comment by chortos@inbox.lv on 15 Jun 2013 at 9:49

GoogleCodeExporter commented 8 years ago
Thanks for the fixes Oleg! I think these are fine. I'll test them for a bit and 
push if everything goes right.

Original comment by g...@chown.ath.cx on 16 Jun 2013 at 12:14

GoogleCodeExporter commented 8 years ago
Haven't actually tested Oleg's fixes, but they look good to me.

But there's some minor code diplication. Is it really needed to duplicate the 
lookup_style function? Maybe it could take a parameter for the fallback style 
or so? The code that deals with '*' in style names is duplicated in two places 
too.

The duplication is really minor, so it's not like I want to reject the patch 
because of that. Since this is new code, it would still be nice if that could 
be fixed, though.

Original comment by nfxjfg@googlemail.com on 16 Jun 2013 at 4:04

GoogleCodeExporter commented 8 years ago
Sorry, I should've explained that. I thought about it and came to the 
conclusion that this was the best way:

The differences between the two functions are the return value type, default 
value, error message and parsing quirks. I actually made this decision before 
the second patch, but the second patch only confirms it by adding more parsing 
quirk differences. The lookup_style code could be unified, but at the expense 
of several added conditionals. The return types could be unified, but at the 
expense of extra logic in calling sites. Ultimately, there's quite little 
shared code (because there's generally quite little code), so a unified version 
would in large part consist of just logic to switch between the two behaviours. 
I'd like to avoid the code duplication, but it just seems like an overall 
better choice this time.

Original comment by chortos@inbox.lv on 16 Jun 2013 at 7:51

GoogleCodeExporter commented 8 years ago
OK then.

Original comment by nfxjfg@googlemail.com on 16 Jun 2013 at 10:31

GoogleCodeExporter commented 8 years ago
Cherry picked your branches in a branch named pullrequest, waiting for greg 
approval.

Original comment by nfxjfg@googlemail.com on 22 Jun 2013 at 5:42

GoogleCodeExporter commented 8 years ago
Pushed. Should be fixed.

Original comment by nfxjfg@googlemail.com on 23 Jun 2013 at 1:10