[MPlayer-dev-eng] [PATCH] fix SAMI parsing

[Howard Chu]

Reimar Döffinger wrote:
> On Sun, Jun 06, 2010 at 01:18:11PM -0700, Howard Chu wrote:
>> @@ -143,7 +146,7 @@
>>   	    }
>>   	    break;
>>
>> -	case 1: /* find (optionnal) "<P", skip other TAGs */
>> +	case 1: /* find (optional) "<P", skip other TAGs */
>>   	    for  (; *s == ' ' || *s == '\t'; s++); /* strip blanks, if any */
>>   	    if (*s == '\0') break;
>>   	    if (*s != '<') { state = 3; p = text; continue; } /* not a TAG */
>
> Speeling fixes don't belong in this patch, and since you have commit acces
> you could just commit them straight away (to reduce the effort, you can have
> a separate checkout where you do such trivial changes, commit them and then
> let svn merge them back into your development tree. Or git-svn if you get
> along well with git...).

OK, committed the spelling fix on its own.

>> @@ -161,17 +164,26 @@
>>
>>   	case 3: /* get all text until '<' appears */
>>   	    if (*s == '\0') break;
>> -	    else if (!strncasecmp (s, "<br>", 4)) {
>> +	    else if (!strncasecmp (s, "<br>", 4) ||
>> +	    	!strncasecmp(s, "<br/>", 5) ||
>> +			!strncasecmp(s, "</P>", 4)) {
>
> /p instead of /P, since if br is lowercase...
> I guess<br />  might be quite common as well?

Perhaps just strncasecmp("<br", 3) then, and s=strchr(s, '>') to advance past 
this.

> The indentation is supposed to be 4 spaces with each group of
> 8 spaces replaced by a tab btw.

OK.
>
>> +	    else if (!strncasecmp (s, "&#", 2)) {
>> +		uint32_t c = strtol(s+2,&s, 0);
>> +		uint8_t tmp;
>> +		PUT_UTF8(c, tmp, *p++ = tmp;)
>> +		sub_utf8 = 1;
>
> I have some doubts setting this in the middle will work
> reliably. Any problem with setting it always?

Move the assignment up to the top of the function? Doesn't seem like it can 
cause any harm. OK.

> -subcp should still allow overriding it (at the expense of
> breaking these).
>
>> +		s++; }
>
> Uh, aren't you possibly skipping a NULL byte there?

I guess. OK: if (*s == ';') s++;

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: sub.txt
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100606/bbbcb457/attachment.txt>