[MPlayer-dev-eng] [PATCH] USF support - second time around NOVIRUS
Attila Kinali
attila at kinali.ch
Sat Feb 21 01:33:52 CET 2004
Heyo,
Do you plan to fix those flaws, so we could
commit your USF patch ?
It would be nice to have it in pre4
Attila Kinali
On Sat, 24 Jan 2004 12:28:10 +0100
Attila Kinali <attila at kinali.ch> wrote:
> On Sun, 14 Dec 2003 04:38:41 -0500
> Avery Morrow <ashitaka-san at comcast.net> wrote:
>
> > I have improved on my patch for USF support. It fixes
> > everything that Moritz pointed out, and now supports XML more
> > fully and displays multi-line subtitles properly. I tested it
> > with a somewhat complex USF subtitle file which I've made, and
> > it works perfectly. However, there still might be some
> > Schroedinbug I didn't see.
>
> diff -Naur 1/subreader.c 2/subreader.c
> --- 1/subreader.c 2003-12-14 04:11:35.792026584 -0500
> +++ 2/subreader.c 2003-12-14 04:11:23.621876728 -0500
> @@ -262,6 +262,83 @@
> return current;
> }
>
> +subtitle *sub_read_line_usf(FILE *fd,subtitle *current) {
> + char line[LINE_LEN+1];
> + int i, j, count, skip, start, a1, a2, a3, a4, b1, b2, b3, b4;
> + char *p=NULL;
> + count = 0;
> +
> +while (!current->text[0]) {
> + while (!strstr(line, "<subtitle ")) {
> + if(!fgets(line, LINE_LEN, fd))
> + return NULL;
> + }
> + if (sscanf(line, " <subtitle start=\"%d:%d:%d.%d\" stop=\"%d:%d:%d.%d\"", &a1, &a2, &a3, &a4, &b1, &b2, &b3, &b4) != 8) {
> + mp_msg(MSGT_SUBREADER,MSGL_WARN,"USF decoding doesn't parse XML correctly.\n"); //TODO: allow more than one line per time marker
> + break;
> + }
> + current->start = a1*360000+a2*6000+a3*100+a4/10;
> + current->end = b1*360000+b2*6000+b3*100+b4/10;
> + if (current->end <= current->start)
> + current->end = current->start+200;
> + while (!(p = strstr(line, "<text"))) {
> + if(!fgets(line, LINE_LEN, fd))
> + return NULL;
> + }
> + int len = 0, addline = 2;
> + // detects </text
> + p = &line[0];
> + for (;!(*p=='<' && *(p+1)=='/' && *(p+2)=='t' && *(p+3)=='e' && *(p+4)=='x' && *(p+5)=='t') && *p; p++,len++) {
> + if (*p=='\n' || *p=='\r') {
> + char tmp[LINE_LEN+1];
>
> This is imho a bad idea. This allocates 1KB on the stack
> and might lead to stack overflows.
>
> [...]
> + char temp[LINE_LEN+1];
>
> Another KB on the stack.
>
> + strncpy(temp, current->text[count], curptr-&(current->text[count][0]));
> + temp[curptr-&(current->text[count][0])] = '\0';
> + current->text[count]=(char *)malloc(j-1);
>
> You should do a realloc here -> memleak
>
> + strcpy(current->text[count], temp);
> + count++;
> + curptr=current->text[count]=(char *)malloc (len-j);
> + }
> + continue;
> + }
> + if(skip || eol(line[j]))
> + continue;
>
> What's the intention of the check for eol ?
> You know that eol also matches for '\0' ?
>
> + if(line[j] == '\t') {
> + if(line[j-1] == ' ')
> + continue;
> + else
> + line[j] = ' ';
> + }
> + *curptr=line[j];
> + curptr++;
> + }
> + *curptr='\0';
> + count++;
>
> There should be a check whether count reached SUB_MAX_TEXT somewhere in this loop.
> -> sig11
>
> +}
> + current->lines=count;
> + return current;
> +}
> +
> subtitle *sub_read_line_subviewer(FILE *fd,subtitle *current) {
> char line[LINE_LEN+1];
> int a1,a2,a3,a4,b1,b2,b3,b4;
> @@ -923,10 +1000,14 @@
> {*uses_time=1;return SUB_VPLAYER;}
> if (sscanf (line, "%d:%d:%d ", &i, &i, &i )==3)
> {*uses_time=1;return SUB_VPLAYER;}
> + // If it's XML, it's USF... this is just as bad as the RT detection
> + if (strstr(line, "<?xml"))
> + {*uses_time=1;return SUB_USF;}
>
> IMHO sub_autodetect() should be reworked to use multiple lines for
> detection, but that's something that could be done later.
>
> //TODO: just checking if first line of sub starts with "<" is WAY
> // too weak test for RT
> // Please someone who knows the format of RT... FIX IT!!!
> // It may conflict with other sub formats in the future (actually it doesn't)
> + // ... it doesn't only if it comes last
> if ( *line == '<' )
> {*uses_time=1;return SUB_RT;}
>
> ------------------
>
> Otherwise the patch looks ok and could be applied if those few things
> are fixed.
>
>
> Attila Kinali
>
>
>
>
> --
> egp ist vergleichbar mit einem ikea bausatz fuer flugzeugtraeger
> -- reeler in +kaosu
>
> _______________________________________________
> MPlayer-dev-eng mailing list
> MPlayer-dev-eng at mplayerhq.hu
> http://mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
>
--
egp ist vergleichbar mit einem ikea bausatz fuer flugzeugtraeger
-- reeler in +kaosu
More information about the MPlayer-dev-eng
mailing list