[prev in list] [next in list] [prev in thread] [next in thread] 

List:       xine-devel
Subject:    Re: [xine-devel] Re: [xine-cvs] CVS: xine-lib/src/libspucc
From:       Frantisek Dvorak <valtri () users ! sourceforge ! net>
Date:       2003-12-03 18:22:32
[Download RAW message or body]

Hi Christian,

V St, 03. 12. 2003 v 18:06, Christian Vogler píše:
> Hi Frantisek,
> 
>  On Wed, Dec 03, 2003 at 06:38:35AM +0100, Frantisek Dvorak wrote:
> > > Index: cc_decoder.c
> > > ===================================================================
> > > RCS file: /cvsroot/xine/xine-lib/src/libspucc/cc_decoder.c,v
> > > retrieving revision 1.22
> > > retrieving revision 1.23
> > > diff -u -r1.22 -r1.23
> > > --- cc_decoder.c	18 Sep 2003 18:14:50 -0000	1.22
> > > +++ cc_decoder.c	3 Dec 2003 01:22:05 -0000	1.23
> > > @@ -376,7 +376,7 @@
> > >    *maxh = 0;
> > >  
> > >    renderer->set_font(testc, (char *) fontname, font_size);
> > > -  renderer->set_encoding(testc, NULL);
> > > +  renderer->set_encoding(testc, "iso-8859-1");
> > 
> > I think this changes nothing. Because start of UNICODE (as it's in the
> > xine fonts) is identical to iso-8859-1, iso-8859-1 will be used for NULL
> > encoding (which means "no conversion"). Something doesn't work?
> 
> Before this change, all 8 bit characters would simply be displayed as
> blanks. With this change, they display as they should. If this should
> have been the default, set_encoding() or the conversion in the OSD
> text renderer must be broken at some place.
> 

Yes, you're right. You found a bug.

> Fact is that, unless the encoding is set to iso-8859-1 explicitly, the
> text rendering seems to be restricted to US ASCII. My first idea was
> that perhaps the conversion in the osd_render_text() function, with
> the default encoding, deems any 8 bit character to be invalid, which
> would make it pretty much irrelevant whether the xine fonts start with
> the iso-8859-1 encoding. 
> 
> I don't know when exactly the breakage occurred, because lately I have
> not been tracking the xine CVS very closely. The problem started to
> occur when Gentoo upgraded the ebuild to rc2.
> 

It passes. This code has been quite new in rc2.

> > Previous code didn't need conversion. Anyway, now it's seen in the code,
> > which encoding is used. ;-)
> 
> Which is a Good Thing(tm) anyway. :-) Can you help me check what
> might have gone wrong here? Myself, I don't have a lot of free time
> right now to go on a bug hunt in the conversion code that I do not
> understand very well as of yet.
> 
> One line that looks suspicious to me is in the osd_iconv_getunicode()
> function:
> 
>   } else {
>     /* direct mapping without iconv */
>     unicode = (*inbuf)[0];
>     (*inbuf)++;
>     (*inbytesleft)--;
>   }
> unicode is an uint16_t, whereas inbuf is a char **. On systems where
> char is signed, 8-bit characters get sign-extended in this assignment
> and the high 8 bits end up as 0xff, instead of 0x00. I don't have time
> to verify whether this is the code path actually taken, but it should
> be worth a shot. This line should actually be
> 
> unicode = (unsigned char)(*inbuf)[0];
> 
> if I understand the intent of this code correctly.

Wow, excelent! You already catched it. :-)

I tested current code really doesn't work with characters >127 how
expected and tested your fix will help.

So now there are too possibilities for cc_decoder:
  1) revert the last change in cc_decoder 
      (+maybe add comments about encoding "iso-8859-1")
  2) leave current code and let to do conversion

Good work.

Cheers,
  Frantisek




-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
Does SourceForge.net help you be more productive?  Does it
help you create better code?  SHARE THE LOVE, and help us help
YOU!  Click Here: http://sourceforge.net/donate/
_______________________________________________
xine-devel mailing list
xine-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/xine-devel

[prev in list] [next in list] [prev in thread] [next in thread]