[MPlayer-dev-eng] Re: [PATCH]Unicode support for ASF demuxer

[Zuxy Meng]

Hi,

2006/11/28, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> Hello,
> On Tue, Nov 28, 2006 at 12:03:27AM +0800, Zuxy Meng wrote:
> > -// the variable string is modify in this function
> > -void pack_asf_string(char* string, int length) {
> > -  int i,j;
> > -  if( string==NULL ) return;
> > -  for( i=0, j=0; i<length && string[i]!='\0'; i+=2, j++) {
> > -    string[j]=string[i];
> > +static char* get_ucs2str(const uint16_t* inbuf, uint16_t inlen, char* outbuf,
> > +    uint16_t outlen)
>
> Due to the way it is used, IMO remove the outbuf and outlen and just do
> the malloc in this function.

When should the malloc'ed be freed then? Do we need a file-scope
static pointer var or something?

>
> > +  for (i = 0; i < inlen / 2; i++) {
> > +    uint8_t tmp;
> > +    PUT_UTF8(le2me_16(inbuf[i]), tmp, if (q - outbuf < outlen - 1) *q++ = tmp;)
>
> If you make sure the input for PUT_UTF8 is of type uint16_t it will
> never write more than 3 bytes, and if you do malloc(inlen*2) (assuming
> the multiplication by two can not overflow, so just to be safe you could
> do calloc(inlen, 2)) that check is not needed.

OK!
> > -            print_asf_string(" Title: ", string, contenth->title_size);
> > -       else
> > -         pack_asf_string(string, contenth->title_size);
> > +         mp_msg(MSGT_HEADER,MSGL_V,"%s%s\n", " Title: ", string);
>
> " Title: %s\n" seems to make a lot more sense to me... There are more
> cases like that below.

Good! I wasn't sure if changing old code in a somewhat unrelated way
is acceptable...

-- 
Zuxy
Beauty is truth,
While truth is beauty.
PGP KeyID: E8555ED6