[FFmpeg-devel] [PATCH] Fix colors for DVD subtitles

[Alexandre Colucci]

On 29 avr. 2011, at 19:25, Reimar Döffinger wrote:

> On Fri, Apr 29, 2011 at 10:11:55AM +0200, Alexandre Colucci wrote:
>> 
>> On 27 avr. 2011, at 22:57, Reimar Döffinger wrote:
>> 
>>> On Wed, Apr 27, 2011 at 12:31:14PM +0200, Alexandre Colucci wrote:
>>>> On 26 avr. 2011, at 22:08, Reimar Döffinger wrote:
>>>>>> Just compare the attached screenshots attached to the patch, they speak for themselves. The screenshots are from Con Air btw, not from some handmade source.
>>>>> 
>>>>> I don't doubt that it works for your case, I just don't believe it
>>>>> works for all (even just all commercial) DVDs.
>>>> 
>>>> I haven't seen it not working ever but I am addressing that issue in the attached new patch.
>>> 
>>> Ok, I really should have done this earlier but I decided to
>>> get some actual data.
>>> Below is the list for different DVDs, alpha and brightness (Y),
>>> all 4 entries:
>>> 
>>> f 10 f 10 f d1 0 28
>>> f 10 f eb f 51 0 10
>>> f 10 f 80 f b0 0 10
>>> f 10 f 10 f eb 0 eb
>>> f 10 f 80 f b0 0 10
>>> f 10 f 10 f d1 0 28
>>> f 10 f ea f ea 0 ea
>>> f d7 f 62 f 24 0 b4
>>> f 10 f 7d f d9 0 10
>>> f 10 f eb f 5d 0 5d
>>> f 10 f 7e f eb 0 eb
>>> f 10 f 80 f b0 0 10
>>> f 62 f d7 f 24 0 ea
>>> f 10 f 7b f d1 0 ea
>>> f 7b f 1e f dc 0 10
>>> f 10 f 7b f ea 0 ea
>>> f 10 f 7b f ea 0 ea
>>> f 10 f ea f ea 0 ea
>>> 
>>> General conclusion: Nobody uses alpha.
>> 
>> 
>> The above table is incomplete, I would need the following values for my tests:
>> 
>> - Cr and Cb values
> 
> Almost all are 0x80, 0x80 (i.e. some gray).
> 
>> - colormap values (4 values in the range 0-15)
> 
> I can see if I can dig them up.
> 
>>> Clear exception where the previous code worked better is
>>> f d7 f 62 f 24 0 b4 (from German version of Hellboy).
>> 
>> It is not clear at all, you are comparing Y with RGB. Again the counter example needs to be backed by real values.
> 
> I really don't care about white vs. yellow here so I don't
> think anything but the Y value really matters.
> But anyway, here's alpha, colormap and associated color
> for this:
> f 3 d78080 f 2 628080 f 1 248080 0 0 b48080
> 
>> One last note: if your source of DVDsubs is ripped idx/sub that you downloaded, a lot of it is generated by various tools and are quite different from the originals.
> 
> It's all original DVDs, so I did have a good reason to be too lazy
> to do this.


Based on the DVD specifications regarding assignment of color indexes and our discussion so far, including your examples of DVDs, my first patch was actually better than the second and it is the one that should be considered for checkin.

To recap:

- the meaning of color indexes is : 00 (background), 01 (pattern), 10 (emphasis-1), 11 (emphasis-2). It is more important to support actual commercial DVDs than some homegrown tool's output where the author did not ever read the specifications.

- we need to accept the fact that guess_palette can be wrong. There should really be no guessing, only in an extreme situation. In a future patch dvdsubdec should really look at the extra_data in avctx just like vlc and others do.

- we need to pass a color to guess_palette and use the full scale for maximum contrast. For that reason white is better than yellow because black-white is greater contrast than black-yellow

All of the above is being taken care of in the first patch submitted in this thread.

Alexandre