[Ffmpeg-devel] [PATCH] HD DVD subtitle decoding

Ian Caulfield ian.caulfield
Mon Jan 29 12:46:00 CET 2007


On 27/01/07, Michael Niedermayer <michaelni at gmx.at> wrote:
>
> Hi
>
> On Thu, Jan 25, 2007 at 11:51:04AM +0000, Ian Caulfield wrote:
> > Hi,
> >
> > The attached patches add support for HD DVD subtitles (both 2-bit and
> 8-bit)
> > to the dvdsub decoder. The patches depend on my earlier bugfix/tidy
> patches,
> > which I've reattached as dvdsub1.patch and dvdsub2.patch.
>
> 1 patch per mail is prefered, 2patches (1 functional + 1 cosmetic is ok
> too)
> but reposting all the prerequisites together with 5 new patches really
> makes
> our work hard


Fair enough - I'll wait for the original patches to be applied before I
submit the updated version of this patch.

> I'm not sure about the yuvtorgb code - I lifted this from dvbsubdec.c, but
> I
> > reckon it should probably live somewhere else, though I couldn't decide
> > where.
>
> well choose a filename you like and put it there, copy and paste is not ok
> ideally the yuv2rgb code from libswscale/ should be used / or the code
> should
> be moved into libswscale/ but thats not so important and can be done later


The defines seem to have been copied from imgconvert.c - should I move the
yuv->rgb function there? Which would be an appropriate header to insert the
declaration into?



> this looks duplicated, maybe this can be factored out into its own
> function?


I'll refactor this

something like
> if(big_palette){
>     nb_colors= 256;
>     pal_entry_size= 24;
>     alpha_entry_size= 8;
> }else{
>     nb_colors= 4;
>     pal_entry_size= 4;
>     alpha_entry_size= 4;
> }


> case 0x03:
> case 0x83:
>     /* set palette */
>     if ((buf_size - pos) < pal_entry_size*nb_colors/8)
>         goto fail;
>     for(i=0; i<nb_colors; i++)
>         palette[i]= get_bits(pal_entry_size);
>     break;
> case 0x04:
> case 0x04:
>     /* set alpha */
>     if ((buf_size - pos) < alpha_entry_size*nb_colors/8)
>         goto fail;
>     for(i=0; i<nb_colors; i++)
>         alpha[i]= get_bits(alpha_entry_size);
>     break;
>
> the same can be done with case 0x86/6


I'll have a look into it...

and guess_palette() could then do the yuv->rgb which is IMHO cleaner then
> your code where the 2bit palette is "converted" in guess_palette() while
> the 8bit one is handled while it is read


I'm not keen on this - in the 2bit case, the "palette" in the bitstream is
actually a mapping of the four colours in the subtitle onto the external DVD
16-color palette. Since we don't have access to this palette, we have to
guess our own. For 8-bit subtitles, an actual palette is embedded into the
bitstream, which we can then use. I could rename the guess_palette function
to guess_palette_2bit, and add an "extract_palette_8bit" function which
would do the colourspace conversion and merge in the alpha components?

Ian




More information about the ffmpeg-devel mailing list