[PATCH] Universal VOB subtitle input

Archive of historical development discussions
Discussions / Development has moved to GitHub
Forum rules
*******************************
Please be aware we are now using GitHub for issue tracking and feature requests.
- This section of the forum is now closed to new topics.

*******************************
davidfstr
Enlightened
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [PATCH] Universal VOB subtitle input

Post by davidfstr »

Both snippets look fine. The first one is better commented but relies on other string functions. The second has no comments, but you can tell what it's doing without looking up external documentation.

Could I get someone who is familiar with ports.c and ./configure (that presumably determines HAVE_STRTOK_R) to integrate one of these?
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Re: [PATCH] Universal VOB subtitle input

Post by dynaflash »

I am going to as usual in these libhb matters pass it off to johnAStebbins or jbrjake, maybe eddyg.
User avatar
JohnAStebbins
HandBrake Team
Posts: 5713
Joined: Sat Feb 09, 2008 7:21 pm

Re: [PATCH] Universal VOB subtitle input

Post by JohnAStebbins »

I'll have some time to work on it tomorrow.
User avatar
JohnAStebbins
HandBrake Team
Posts: 5713
Joined: Sat Feb 09, 2008 7:21 pm

Re: [PATCH] Universal VOB subtitle input

Post by JohnAStebbins »

User avatar
JohnAStebbins
HandBrake Team
Posts: 5713
Joined: Sat Feb 09, 2008 7:21 pm

Re: [PATCH] Universal VOB subtitle input

Post by JohnAStebbins »

Here's a little patch to allow soft vobsub tracks in mp4 using the non-standard nero format.
http://handbrake.fr/pastebin/pastebin.php?show=1431
davidfstr
Enlightened
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [PATCH] Universal VOB subtitle input

Post by davidfstr »

JohnAStebbins wrote:Here's a little patch to allow soft vobsub tracks in mp4 using the non-standard nero format.
http://handbrake.fr/pastebin/pastebin.php?show=1431
Interesting. I had to modify test.c a bit to get the CLI to pass it through.
http://handbrake.fr/pastebin/pastebin.php?show=1432

I'm a bit concerned that since the Nero VOB embedding format isn't standard yet, that it won't be recognized on a number of players. Would it make sense to add a warning to the user in the CLI/GUIs?

Testing results:
  • VLC 0.9.8 - recognizes vobsub track and plays it
  • QuickTime 7.6.6 - fails to recognize track
  • iPod Touch / iPhone OS 3.1.2 - fails to recognize track
User avatar
JohnAStebbins
HandBrake Team
Posts: 5713
Joined: Sat Feb 09, 2008 7:21 pm

Re: [PATCH] Universal VOB subtitle input

Post by JohnAStebbins »

Yes, pretty much all apple devices and software will fail. perian might make it work. I tested mplayer and vlc which both work and I suspect several other windows based players will work.

I also thinking that i would add vorbis and theora support to mp4 using the format created by gpac. Not likely to be supported by many commercial players, but then that's the case for vorbis and theora in general.

Thanks for the cli fix. We usually don't do much of that type of checking in the cli, so I didn't check it. The cli is an anything goes kind of tool so that we can test what happens in libhb when you feed it nonsense.
Deleted User 11865

Re: [PATCH] Universal VOB subtitle input

Post by Deleted User 11865 »

FWIW, we already allow MP3 in MP4 which, despite being standard, is about as well supported as VOBSUB-in-MP4…
Deleted User 11865

Re: [PATCH] Universal VOB subtitle input

Post by Deleted User 11865 »

davidfstr wrote:Interesting. I had to modify test.c a bit to get the CLI to pass it through.
http://handbrake.fr/pastebin/pastebin.php?show=1432
I modified your patch very slightly: http://handbrake.fr/pastebin/pastebin.php?show=1433

Same functionality.
Deleted User 11865

Re: [PATCH] Universal VOB subtitle input

Post by Deleted User 11865 »

MacGUI patch for review (to work with j45's VOBSUB-in-MP4 patch): http://handbrake.fr/pastebin/pastebin.php?show=1434

Seems to work in my testing, although I fear I may have forgotten something very obvious/necessary.

Also, as I understand it the code removed by the first chunk of my patch makes any new VOBSUB track "burned in" by default when the container is MP4. I wonder whether leaving it in would be sensible, as many people encoding to MP4 might prefer burned in subs for compatibility.
User avatar
JohnAStebbins
HandBrake Team
Posts: 5713
Joined: Sat Feb 09, 2008 7:21 pm

Re: [PATCH] Universal VOB subtitle input

Post by JohnAStebbins »

Committed libhb, cli, and lingui parts for vobsubs in mp4 http://trac.handbrake.fr/changeset/3325

Rodeo, I think it would be a good idea to make the default on mac be burned in since nothing apple makes supports this format.
Deleted User 11865

Re: [PATCH] Universal VOB subtitle input

Post by Deleted User 11865 »

FWIW, Perian doesn't do anything with VOBSUB in MP4 (and neither does Plex, IIRC).
eddyg
Veteran User
Posts: 798
Joined: Mon Apr 23, 2007 3:34 am

Re: [PATCH] Universal VOB subtitle input

Post by eddyg »

John & Rodeo,

Thanks again for making/doing this work.

Cheers, Ed.
Deleted User 11865

Re: [PATCH] Universal VOB subtitle input

Post by Deleted User 11865 »

eddyg wrote:John & Rodeo,

Thanks again for making/doing this work.

Cheers, Ed.
Let's not forget David ;-)
TedJ
Veteran User
Posts: 5388
Joined: Wed Feb 20, 2008 11:25 pm

Re: [PATCH] Universal VOB subtitle input

Post by TedJ »

Rodeo wrote:Let's not forget David ;-)
Geez, aside from writing most of the code, what did David actually do? :lol:
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Re: [PATCH] Universal VOB subtitle input

Post by dynaflash »

Macgui implementation committed as http://trac.handbrake.fr/changeset/3326 which is Rodeo's patch with the burned in default put back in. Thanks Rodeo!
Deleted User 11865

Re: [PATCH] Universal VOB subtitle input

Post by Deleted User 11865 »

dynaflash wrote:Macgui implementation committed as http://trac.handbrake.fr/changeset/3326 which is Rodeo's patch with the burned in default put back in. Thanks Rodeo!
Thanks.

IMO "burned in" default makes sense for all platforms - aside from VLC and MPlayer, and possibly a few additional Windows-based or FFmpeg-based solutions, I wonder whether VOBSUB-in-MP4 is that much better supported on other platforms (plus, there are Apple device users on other platforms, too).

Edit: I had a feeling the "burned in default" part required more changes - right now, if you add several VOBSUB tracks one after the other, they all get their "burned in" checkbox checked, so you end up with several "burned in" tracks - when you encode, libhb ignores all tracks except the first.

Should be a reasonably easy fix, so I'll submit a patch if I can, but feel free to beat me to it :-)
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Re: [PATCH] Universal VOB subtitle input

Post by dynaflash »

heh, I see the part you took off I should put back in. My bad ... I will fix it.
Deleted User 11865

Re: [PATCH] Universal VOB subtitle input

Post by Deleted User 11865 »

Nice - HB can grok its own output thanks to FFmpeg, is also finds VOBSUB tracks in MP4 :-)

Unfortunately, mkvtoolnix does not.
eddyg
Veteran User
Posts: 798
Joined: Mon Apr 23, 2007 3:34 am

Re: [PATCH] Universal VOB subtitle input

Post by eddyg »

TedJ wrote:
Rodeo wrote:Let's not forget David ;-)
Geez, aside from writing most of the code, what did David actually do? :lol:
precisely!

(thanks David :)
User avatar
JohnAStebbins
HandBrake Team
Posts: 5713
Joined: Sat Feb 09, 2008 7:21 pm

Re: [PATCH] Universal VOB subtitle input

Post by JohnAStebbins »

Rodeo wrote:Nice - HB can grok its own output thanks to FFmpeg, is also finds VOBSUB tracks in MP4 :-)

Unfortunately, mkvtoolnix does not.
Hmm, the palette information in mp4 is stored differently than it is for mkv. So there needs to be a different extradata parser that depends on the file format. It looks like ffmpeg is also just passing this information along in codec->extradata. But in this case, the data is formated as 16 4-byte palette entries instead of the whole idx text. The palette is YCbCr (same as original palette from dvd) instead of RGB. The byte ordering is: 0, Y, Cb, Cr.

I also noticed that we are not preserving the width and height information from the vobsub tracks. This width and height are suppose to be the original width and height of the source that the vobsubs were taken from. The position and size information that is contained in the vobsub entries is all relative to this original size. So we should really be reading this info and passing it through. In mkv, this information is in the codec-extradata with the palette info. In mp4, it looks like ffmpeg is putting this in codec->width and codec->height.
davidfstr
Enlightened
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [PATCH] Universal VOB subtitle input

Post by davidfstr »

Patch to fix the VOB palette colors when reading from an MP4:
http://handbrake.fr/pastebin/pastebin.php?show=1439

Strangely, I seemed to be getting the correct colors even without this patch, even though decvobsub was complaining. Need another tester to make sure this works.
User avatar
JohnAStebbins
HandBrake Team
Posts: 5713
Joined: Sat Feb 09, 2008 7:21 pm

Re: [PATCH] Universal VOB subtitle input

Post by JohnAStebbins »

mplayer ignores the palette. vlc pays attention to the palette in mkv only because I fixed it when I was testing vobsubs in handbrake when they were originally added. I don't know if vlc reads the palette from mp4. They may just be using a default palette.
Deleted User 11865

Re: [PATCH] Universal VOB subtitle input

Post by Deleted User 11865 »

JohnAStebbins wrote:vlc pays attention to the palette in mkv only because I fixed it when I was testing vobsubs in handbrake when they were originally added.
VLC 1.0.5 for Mac OS X still ignores the palette in MKV (though it does display the correct colors from DVD sources).
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Re: [PATCH] Universal VOB subtitle input

Post by dynaflash »

Rodeo wrote:
dynaflash wrote:Macgui implementation committed as http://trac.handbrake.fr/changeset/3326 which is Rodeo's patch with the burned in default put back in. Thanks Rodeo!
...I had a feeling the "burned in default" part required more changes - right now, if you add several VOBSUB tracks one after the other, they all get their "burned in" checkbox checked, so you end up with several "burned in" tracks - when you encode, libhb ignores all tracks except the first.

Should be a reasonably easy fix, so I'll submit a patch if I can, but feel free to beat me to it :-)
Fixed: http://trac.handbrake.fr/changeset/3328
Post Reply