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
RetroSearch is an open source project built by @garambo | Open a GitHub Issue
Search and Browse the WWW like it's 1997 | Search results from DuckDuckGo
HTML:
3.2
| Encoding:
UTF-8
| Version:
0.7.4