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

[Reimar Döffinger]

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...).

> @@ -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?
The indentation is supposed to be 4 spaces with each group of
8 spaces replaced by a tab btw.

> +	    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?
-subcp should still allow overriding it (at the expense of
breaking these).

> +		s++; }

Uh, aren't you possibly skipping a NULL byte there?