-
Notifications
You must be signed in to change notification settings - Fork 227
Enabling MEncoder ASS subtitle support for embedded subtitles. #60
Comments
I have tried your patch, but I don't notice a difference on any of my test streams. Can you supply a movie sample where we can see the difference? |
I did some more testing with various combinations of old and new mencoder builds (SB22, SB32, SB35 and r35007), several movies with external and embedded subtitles, with and without the proposed patch, with the default settings and the ones that Happy-Neko presented. I can now confirm that the patch indeed fixes the rendering of some of the movies with SSA embedded subs. Without the patch, subs do not render correctly as in the screenshots presented in this issue, not even with different settings in PMS or by using mencoder-SB22. At the same time the patch left other movies and subtitles intact. Since the patch appears to be correct, I have outcommented the lines in commit 503eb25. Closing this issue accordingly. |
I don't have more files to test with; as written above I tested it with all combinations I could come up with and in all combinations the patch worked as expected. You're welcome to run more tests as I only have a very limited set of test files.
The code snippet affects UTF-8 embedded and non-embedded subtitles. Note that inside the if-statement is the only place where the As for Hebrew, that is indeed an issue, but it is a different issue, I think. |
While I don't have a short/small test video, I can present the following to you as a test: http://www.nyaa.eu/?page=torrentinfo&tid=324868 This is the video that I took the screenshots from, the issue presents itself within the first minute or so.
This is completely correct, I was a little unsure as to why it was picking out both the 'embedded' and UTF-8 values as exclusion criteria. I vaguely remember a little over a year ago that mplayer/mencoder didn't handle these properly all the time, but I'm pretty sure that's no longer an issue (at least from what I've seen since 1.20 of PMS).
The problem is, as shown in the screenshot, if there are complex subtitles on the screen they tend to format in that weird fashion which makes them very difficult to read. I've tried every combination of checking and unchecking those boxes and also leave 'UTF-8' as my standard subtitle format and have had no success with displaying subtitles correctly on any of my anime MKVs that use ASS formatting. The modification is the only way I've been able to make things display properly. I'm curious as to why this breaks with Hebrew as I've seen Chinese and Japanese subtitles render properly. Do you guys have a test file as well, I'd like to do my own research as well if you guys don't mind. |
DLNAMediaSubtitle.EMBEDDED most likely is assigned to embedded SRTs only https://github.com/ps3mediaserver/ps3mediaserver/blob/master/src/main/java/net/pms/dlna/MediaInfoParser.java#L361 All my mkvs report Codec ID:
Can you share your mkv+ssa sample affected by this patch?
I am not sure there are any real differences between them,
I don't have many animation videos but few test samples displays exactly like they do on PC. On somehow related note from my experience most of their releasers compete in crazy "make most incompatible file ever" game: flac tracks, crazy reference frame counts, 10bit x264, segmented mkvs etc. I don't really know what theses people are thinking.
I give it a try. |
Yeah, I removed that from my comment a few minutes later because I realized how stupid that sentence was! But from what I understand of ASS, it's a subset of SSA/newer standard of SSA. So a lot of older applications don't always support both (and I'm not sure how compatible the new version is with the older versions).
People love seeding them Animes!
Was that with the '-ass' flag or without? |
With
So either muxed SSA subtitles are not Mediainfo for this file
|
I have a strong feeling it might be the UTF-8 check because I think that's what was causing the inter-boolean statement to fail of the removed code. I guess I was fixing a symtom and not the actual problem, especially since I just stumbled across this which indicates that all subtitles are converted to UTF-8: |
UTF8 code looks like total disaster. There are only two places where it can be set to true:
So it looks like commented snippet https://github.com/XenoPhex/ps3mediaserver/commit/538f87bc960042ecb58cc1cf1ef72ee841122dda should never be triggered in the first place. We need to properly fix this mess instead of sticking with workarounds. |
I agree, I submitted the patch under the assumption that the encoding was not UTF8, but I can see now that it is not the case. Should I close this out and let you guys handle the rest? Or should I at least create a separate issue? |
Definitely. It sounds wrong that SSA subs could only be non-UTF-8. |
I suggest to drop all UTF8 checks for embedded subs. They are pointless. |
Looks like the method I suggest we relabel Incidentally, the suggested replacement immediately demonstrates why the patch is indeed correct. It would transform that particular bit of code into:
This expression is always true; that is why outcommenting makes no difference whatsoever to the rest of the if-statement. |
I completed first iteration of DLNAMediaSubtitle refactoring. Before testing do not forget to:
In the future I plan to remove |
Looks like you had a good time refactoring the whole lot. |
There were NullPointerExceptions, I fixed those in 0a7a4d3. |
Great! |
Awesome work, Happy-Neko! :-D |
Thanks! |
Oh wow, lots'a things just changed! Let me run this against a couple different set files and let me get back to you. I think I noticed some funky behavior last night with MP4 subs, after your first set of changes. Let me confirm if that's still true. But everything else looked like it was working! I'll try and post my results in an hour or so. |
Here's a fun one, I'm getting the following error every time I go into a #--TRANSCODE--/*.ogm. I get the following error:
|
Please attach mediainfo for this file. |
In the process, apparently you have to F****** pay for MediaInfo on OS X now, so I'm just trying to track down an older version. |
Screw it, I'll just pay the 99 cents. Worth it, right?
|
Interesting, mediainfo does not report Codec ID for SRTs in ogm. And one more thing: check if your ps3ms have libmediainfo successfully loaded (trace tab). |
I had to break it into two parts (Winrar) since the file was too big: I'll keep these up until the end of the week, just in case the rest of your team wants a copy, after that i'll drop them. |
Seems that subtitles inside an MP4 container aren't working. My test video (sorry for the large size): http://www.nyaa.eu/?page=torrentinfo&tid=260321 The weirder thing is that not only does it not matter what audio or subtitle you select it, it only seems to play English Audio (aid 1) with no subtitles selected. The audio issue is probably a separate bug, but I'll try and track that down once I figure out what's bugging out with the subtitles. What confuses me is that the following command does work as intended:
Debug Log:
|
I've traced the previous problem to the '-ass' being added into the mencoder string. If it's removed, it works properly. I'll file a separate issue for the Audio problem since that looks like an off by 1 error. |
Both .ogm and .mp4 samples work now. Though I noticed some VobSub flickering with mencoder SB22. |
Lots of changes! So I'll go at them in points:
I see this in the log when I first enter the folder:
And then I see this from the debug log:
MediaInfo for file:
Some notes on this issue:
|
Because of all the fixes, I suddenly got strange (yet correct) audio and subs combos (e.g. English audio with English subs). I have modernized the priority defaults in commit 6f6f281 to compensate for this. |
I've updated to 6f6f281 and most things work properly when I run with the following setup (after I killed by DB & conf file): However, I've encountered three separate issues so far:
I can make it looks something like this:
or even like this:
I've even seen cases of multiple #TRANSCODE folders in the same directory. I've also seen it lists files for (according to the example above) from DDD in CCC and then after a few seconds refreshes and show the correct files. |
This is expected behaviour. See example PMS.conf for explanations:
I have identified the bug but the "proper" fix requires significant refactoring similar to subtitles' one. Please provide mediainfo's |
I encountered a problem. When I play a specific testmovie with "Transcoding Settings > MEncoder > Audio/subtitles language priority" set to "en,off;,", PMS decides to use both MEncoder and tsMuxeR and the PS3 shows a black screen, resulting in an error "Media Server Error: An HTTP error (-2147284704) has occurred.". When I do the same with "en,en;," as setting, the movie plays correctly with English audio and English subs. (For reference: in PMS 1.54.0 this movie plays with Japanese audio and no subs, no matter what the setting.) This is what FFmpeg says in the debug.log:
|
@Raptor399, can you upload somewhere your test file? |
Sure: Expires in 30 days. |
I hope this bug is fixed now.
With Mencoder engine every combination of subs/audio works. Default [Mencoder] option with autoselected (by priority) tracks works too. |
I understand this, but in this case it's down-mixing a 5.1 stream into a 2.0 stream when you have everything but LPCM selected which is the real issue for me. If the intent of having 'Keep DTS track' is to keep it as it's original form without re-encode, this means it should keep a 5.1 DTS stream as a 5.1 DTS stream and not down-mix it to 2.0 DTS stream. What I was trying to get at, and I apologize if I was a unclear, was that the only time I was able to get the 5.1 into the system was when I had LPCM checked and DTS unchecked, even though the original stream was a DTS 5.1 stream. Which is what was confusing me if it's purely remuxing and not transcoding. The only reasons, to me at least, this would make sense is if the PS3 doesn't support 2.0 DTS during streaming or if whatever is being used to remux the audio doesn't support 5.1 streams.
As of d1b0b04 I haven't seen an issue! So you guys must have caught the problem already. If I see it again, I'll try and get the channel information to you. I also see that you guys fixed the folder issue too. Props! |
It is original DTS just marked as PCM stereo to fool PS3. You should see DTS logo on receiver. |
@Happy-Neko: |
ps3ms should not try to outsmart user. I'm not a complete idiot and can decide what engine I want to force from TRANSCODE folder and what subtitle/audio track to select. And I expect exactly this engine to be used. Instead we need to implement better engines/players priority system. Player should "know" what input/output formats it can process and tell PlayerFactory if requested transcoding (or transformation) is not possible. For example tsmuxer cant process h264 with variable framerate or decrease reference frames count and should refuse to do this in favour of next registered engine (for ex. mencoder). Your requirement to force subtitles for non-native languages may be part of this system. |
I agree with you. But lot of people are only "users" so #TRANSCODE# folder irritates them because they don't understand for what it is used. Also on some renderes alternatives in this folder are cut so you can't see the difference between more combinations listed there(like PanTV etc.) With the second paragraph I totally agree. |
I consider myself being a simple user for the configuration of the transcoding engines. I keep the default settings and use the transcode folder when I need a 'special' combination of encoder/audio/subtitles, as for me it is a lot easier to understand then trying to set up pms to be smart enough to always do what I want by default. I watch most english films without subs, e.g. for True Grit I need subs as I've got no chance to understand a Texan Jeff Bridges. On the other hand I found following rather confusing (don't know if this is still all true):
Amen |
I disagree. There is nothing hard to understand in "Please select audio/subtitle track from menu if you unsatisfied with what plays by default". In fact every DVD player since forever has subtitle/audio selection menus and nobody dies :) Maybe we can rename #TRANSCODE# to something more friendly like Other languages or Select audio/subs. But manual selection system itself is rock solid and very easy to use out of the box. This is very important. Tinkering with languages priorities is something less than 5% of users will do and I think they'll be technical enough to understand logic behind Transcode folder too.
I already have rough idea how this can be done but it'll require significant time. I don't want to start this process until enough free time can be allocated. Also I have to finish audio refactoring first but it is hard to motivate myself when all related obvious bugs are already fixed and the work needs to be done just to avoid leaving another half-finished v2 implementation and code duplication (like parser and reques v1/v2).
This is true. And this is exact reason why |
I agree but I suppose others not:-) I everyday read complains how to choose alternative subs or audio tracks and people wrote what the hell is #TRANSCODE# folder:-) Many people are using SB builds which hides this folder so they never saw this folder on renderer. They will only set audio/video priority and voila, everything is working out of the box without browsing to #TRANSCODE# folder (visible or invisible). Only few are using PMS for DVD so choosing titles or chapters is very rare in my opinion. |
When we'll merge with mlx, all folders (also the currently not working transcode folder) can be renamed in the tree |
Thanks for the tips. My point was not so much that there isn't a working option combo to get the movie playing. It was more that PMS 1.54.0 and the latest snapshot react differently in determining their choice of MEncoder vs. MEncoder + tsMuxeR, even with the exact same settings. I'm not sure that change in behavior was intended? As for the other discussion: I'm not an expert user at all and I would like to see PMS work great (and intelligent) out of the box with no or minimal configuration, whilst still allowing experts to make their tweaks. A better engine election system (or improved .confs) could definitely help here because we know exactly which engines are best suited for which MediaInfo. Perhaps we should simply introduce a new getPlayer() like this:
|
I completely agree, I know a few people that got confused the first time they used this and either stumbled upon the language selection by accident or I had to tell them where to look. So changing it would be a bit friendlier to new users. Also, while I agree it's a good system to select langauge/subtitles/encoder, I think it could be tweaked by changing the naming scheme it displays. In my 8th MS Team example, here are the names of some of the streams:
Now 3 different audio streams with 4 different subtitle tracks (include no subs) means that there are at least 12 different selections for MEncoder alone. Now I understand this is an unusual case for a video, not just having this many audio/subtitle tracks, but also having extremely long title names. But it would be nice if these selections could be formatted in a way that was easier to read. Especially since the PS3 has to scroll after the first 1/3 of most of these names. In this case it takes a few seconds to read the entire name for each file and longer to figure out which selection you actually want. Now I don't know the best way to format the name, especially since in a lot of cases where the 'Title' information (or 'Language, more info' depending on the container) is not set on the track. But if given the choice, I'd say that in most cases, if a track has title information, it's easier to have that ahead of all other information. I'd even go so far as to say that these names should only show the track properties in cases where the title is not set. So in the examples, it would be:
Now in this case, the titles are very descriptive and tell you not only the language, but the stream information as well. But in the case of the Eureka Seven Ao example, it would cause a bit of confusion as the the item to select the japanese audio with english audio would look like this:
So I can't really suggest this as a good idea unless you add an additional option to always show language and/or stream information when you have these selections making the Eureka Seven example look like this:
But again the problem arises that the name now becomes very long and difficult to read. Maybe the language should be set before the slash and '[Advanced] SubStation Alpha' should be renamed 'ASSA' to reduce some space. Also moving '[Mencoder]' to the end would make things a bit quicker to select since all the MEncoder items are always together, same with tsMuxeR. I'm yammering at this point. But what I'm getting at is that this section, although easy to use, is a bit of a pain in the ass to read if your titles are a more then a few characters long or if you have a lot of different tracks per file. |
Better to have max info possible by default. If it is too long for someone, it is possible to shorten it in renderer.conf: |
#--TRANSCODE--# folder renamed to #- Language and player select -#
Yes, and also add renderer into the mix. Basically engine choice is determined by thee factors:
|
It is hard to name it somehow to show every possibility but in short format (player, language, chapters, ...) |
Actually, renderers are pretty well covered. Improvement in that section can still be achieved; it means we need to start figuring out "supported" lines for all .confs instead of the lists of extensions that most .confs use. |
Issue closed because reported bug is fixed. |
MPlayer/MEncoder has had support for this for a while now and I'm wondering why it's been blocked. To show the difference during playback I ran this under MPlayer (the only difference between the runs was the '-ass' flag):
http://s14.postimage.org/nfojh6v69/Non_ASS_Subtitles.png
http://s16.postimage.org/bcwjfb7ad/ASS_Subtitles.png
Most Anime (usually in MKV containers) or other subtitled shows tend to contain embedded subtitles that display in the maner shown in the first screenshot and make it very difficult to watch. I've made the following change (I guess I should have made this a pull request?) that makes the subtitles render as they do in the second screenshot:
https://github.com/XenoPhex/ps3mediaserver/commit/538f87bc960042ecb58cc1cf1ef72ee841122dda
I know I've had to make this change on a few of my friend's copies of PMS and I think it's worth looking at if you guys have the time.
The text was updated successfully, but these errors were encountered: