dvdsub encoder: respect palette when it makes sense
Hi. The current version of the dvdsub encoder uses the alpha channel of the palette to weight the most important colors, and then completely ignores the palette. In particular, the alpha values it puts in the stream are hardcoded arbitrary values. The attached patch fix that to a certain extent: when there is exactly one rectangle and the palette has exactly 4 colors, the colors are kept in their original order and the alpha channel of the palette is copied to the stream. With this patch, it becomes possible to use the dvdsub encoder to produce a VOBSUB pair of file that mplayer can understand. Regards, -- Nicolas George
On Sun, Mar 01, 2009 at 10:45:38AM +0100, Nicolas George wrote:
Hi.
The current version of the dvdsub encoder uses the alpha channel of the palette to weight the most important colors, and then completely ignores the palette. In particular, the alpha values it puts in the stream are hardcoded arbitrary values.
The attached patch fix that to a certain extent: when there is exactly one rectangle and the palette has exactly 4 colors, the colors are kept in their original order and the alpha channel of the palette is copied to the stream.
your patch looks like a hack ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
Le primidi 11 vent?se, an CCXVII, Michael Niedermayer a ?crit?:
your patch looks like a hack ...
I agree, it does look like a hack; the reason is probably that the code it modifies is itself a hack. To be more precise, DVD subtitles are four-color pixmaps; the current code accepts any number of colors and maps them semi-randomly to the four slots; it then ignores the provided palette. My patch kicks in when the user provides exactly four colors, and keeps them as is. I could submit a patch that makes the encoder fail if the provided number of colors is not 4; I think it would make much more sense that the current behavior. But this is a change that may possibly break currently working code, and I would not do it without advice. Regards, -- Nicolas George
On Sun, Mar 01, 2009 at 11:52:21PM +0100, Nicolas George wrote:
Le primidi 11 vent?se, an CCXVII, Michael Niedermayer a ?crit?:
your patch looks like a hack ...
I agree, it does look like a hack; the reason is probably that the code it modifies is itself a hack.
To be more precise, DVD subtitles are four-color pixmaps; the current code
dvdsubdec seems to support 8bit also in the other mail you spoke about makin them playabe in mplayer, this actually is of almost no relevance, it rather matters if they ae playable on hardware dvd players / if they are compliant tto he specs [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes
Le duodi 12 vent?se, an CCXVII, Michael Niedermayer a ?crit?:
dvdsubdec seems to support 8bit
I found some information here: http://www.mpeg.org/MPEG/DVD/Book_B/Subpic.html and there: http://dvd.sourceforge.net/dvdinfo/spu.html # 4 colors out of 16 color palette (4 colors are determined once per PGC). # 4 out of 16 contrast values In practice, that means: - there is a 16-colors palette in the DVD title's IFO file or the VOBSUB idx file; - each subpicture packet is a 4-color pixmap; - each subpicture packet has two fields that give, for the four color cells, the alpha value and the palette entry number (lines 164-167 of the current code). The 8bit stuff is for HD-DVD subtitles, added by revision 9788. The encoder does not support HD-DVD subtitles.
also in the other mail you spoke about makin them playabe in mplayer, this actually is of almost no relevance, it rather matters if they ae playable on hardware dvd players / if they are compliant tto he specs
You are perfectly right in principle, and I did not express myself correctly. I can make a VOBSUB file that mplayer can play with either the current version of my patched version; supposedly, other compliant players would play it too. But the colors (or, in mplayer case, the gray levels) are randomly permuted. This is both what I can see in mplayer and what I understand in the source code. The purpose of my patch was to stop the random permutation of colors. I now see a better way to do it, and will shortly submit a patch implementing it. Regards, -- Nicolas George
Hi. Here is a new version of the patch. It does two things: - It prints a warning when a subtitle has more than 4 colors or when one of the 4 main colors is beyond 16. - It copies the palette and alpha information to the encoded data instead of hardcoded arbitrary values. (It also adds an AVCodecContext parameter, just to have a proper context for the warning.) Regards, -- Nicolas George
On Mon, Mar 02, 2009 at 02:41:10PM +0100, Nicolas George wrote:
Hi.
Here is a new version of the patch. It does two things:
- It prints a warning when a subtitle has more than 4 colors
ok i suspect
or when one of the 4 main colors is beyond 16.
the way i understand it it should reject it if any of the 4 colors have a palette entry that cant be represented
- It copies the palette and alpha information to the encoded data instead of hardcoded arbitrary values.
(It also adds an AVCodecContext parameter, just to have a proper context for the warning.)
should be a seperate patch ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The greatest way to live with honor in this world is to be what we pretend to be. -- Socrates
Le duodi 12 vent?se, an CCXVII, Michael Niedermayer a ?crit?:
the way i understand it it should reject it if any of the 4 colors have a palette entry that cant be represented
As I favour offensive programming over defensive programming?, I agree that a real error would be better than just a warning. That was just a change of behavior I was not ready to make without advice. But in that case, I think both warnings should be errors, as they are two manifestations of the same problem: anything that is not 4 colors out of 16 can not be coded as a DVD subpicture.
should be a seperate patch ...
Ok. I will resubmit as soon as I get advice about the rest. As for the testing issue: testing on a real DVD player would clearly be the best option. Unfortunately, I think there are no DVD authoring tools that use libavcodec to encode the subtitles. And writing new code to produce a test sample would be no good, as that new code itself would not have been properly tested. Under these circumstances, I think it is reasonably safe to assume that if mplayer displayed DVD subtitles black with white outline when they should be white with black outline, we would have noticed by now. Regards, -- Nicolas George 1?: I'm not sure the terms are completely standard. For me: - offensive programming: assert(x > 0); - defensive programming: if (x <= 0) x = 1;
On Tue, Mar 03, 2009 at 10:22:14AM +0100, Nicolas George wrote:
Le duodi 12 vent?se, an CCXVII, Michael Niedermayer a ?crit?:
the way i understand it it should reject it if any of the 4 colors have a palette entry that cant be represented
As I favour offensive programming over defensive programming?, I agree that a real error would be better than just a warning. That was just a change of behavior I was not ready to make without advice.
But in that case, I think both warnings should be errors, as they are two manifestations of the same problem: anything that is not 4 colors out of 16 can not be coded as a DVD subpicture.
should be a seperate patch ...
Ok. I will resubmit as soon as I get advice about the rest.
rest? please be more specific [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Good people do not need laws to tell them to act responsibly, while bad people will find a way around the laws. -- Plato
Le septidi 17 vent?se, an CCXVII, Michael Niedermayer a ?crit?:
rest? please be more specific
Whether both warnings should be errors, or just one, or... PS: my stupid telco mistakenly disconnected me, sorry if I can not answer quickly to my mail nor sign it. Regards, -- Nicolas George
On Tue, Mar 10, 2009 at 04:40:40PM +0100, Nicolas George wrote:
Le septidi 17 vent?se, an CCXVII, Michael Niedermayer a ?crit?:
rest? please be more specific
Whether both warnings should be errors, or just one, or...
both should be errors [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is dangerous to be right in matters on which the established authorities are wrong. -- Voltaire
On Mon, Mar 02, 2009 at 10:32:07AM +0100, Nicolas George wrote:
Le duodi 12 vent?se, an CCXVII, Michael Niedermayer a ?crit?:
dvdsubdec seems to support 8bit
I found some information here: http://www.mpeg.org/MPEG/DVD/Book_B/Subpic.html and there: http://dvd.sourceforge.net/dvdinfo/spu.html
# 4 colors out of 16 color palette (4 colors are determined once per PGC). # 4 out of 16 contrast values
In practice, that means:
- there is a 16-colors palette in the DVD title's IFO file or the VOBSUB idx file;
- each subpicture packet is a 4-color pixmap;
- each subpicture packet has two fields that give, for the four color cells, the alpha value and the palette entry number (lines 164-167 of the current code).
The 8bit stuff is for HD-DVD subtitles, added by revision 9788. The encoder does not support HD-DVD subtitles.
also in the other mail you spoke about makin them playabe in mplayer, this actually is of almost no relevance, it rather matters if they ae playable on hardware dvd players / if they are compliant tto he specs
You are perfectly right in principle, and I did not express myself correctly. I can make a VOBSUB file that mplayer can play with either the current version of my patched version; supposedly, other compliant players would play it too.
other? i do not thing the authors of the code on mplayet had access to the dvd specs. actually iam pretty darn sure they did not. I really think testing should happen with some player that was written by people who saw the whole dvd specs. That is for changing the encoder output in non trivial ways of course not for just telling the user that the input is invalid or trivial changes ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When the tyrant has disposed of foreign enemies by conquest or treaty, and there is nothing more to fear from them, then he is always stirring up some war or other, in order that the people may require a leader. -- Plato
participants (2)
-
michaelni＠gmx.at -
nicolas.george＠normalesup.org