[MPlayer-dev-eng] [PATCH] subcp_recode changes

[Reimar Döffinger]

Hello,
the attached patch changes subcp_recode in the following ways:
1) It allocates a buffer if 4* the input string before iconv. This wastes
   quite a bit of memory but is IMHO better than the current, only 512
   bytes big static buffer.
2) after failing to convert a line it will continue with the next
   instead of not loading the subtitle at all. This may result in a lot
   of "SUB: error recoding line." messages if -subcp is set wrong but
   seems better to me than failing completely just because of one bogus
   character.
3) check for icdsc == (iconv_t)(-1) and return in that case without
   doing anything.
4) with all these changes I do not see why subcp_recode1 would be needed
   and replaced it with subcp_recode. Somebody please test with an ogg
   file with included subtitles, I do not have one.
   This function must be changed anyway since it assumes that the
   sub->text[l] buffers are 512 bytes large while in demux_ogg they are
   only allocated with 128 bytes.

Also check that the following is correct:
I did not check for an integer overflow at
oleft = 4 * ileft;
because that should not cause real problems, and oleft + 1 should not be
possible to overflow because of the previous multiplication.
Please test and comment.

Greetings,
Reimar Döffinger
-------------- next part --------------
Index: subreader.c
===================================================================
--- subreader.c	(revision 20218)
+++ subreader.c	(working copy)
@@ -1116,69 +1116,35 @@
 	}
 }
 
-#define ICBUFFSIZE 512
-static char icbuffer[ICBUFFSIZE];
-
-static subtitle* subcp_recode (subtitle *sub)
+subtitle* subcp_recode (subtitle *sub)
 {
 	int l=sub->lines;
 	size_t ileft, oleft;
 	char *op, *ip, *ot;
+	if(icdsc == (iconv_t)(-1)) return sub;
 
 	while (l){
-		op = icbuffer;
 		ip = sub->text[--l];
 		ileft = strlen(ip);
-		oleft = ICBUFFSIZE - 1;
+		oleft = 4 * ileft;
 
+		if (!(ot = malloc(oleft + 1))){
+			mp_msg(MSGT_SUBREADER,MSGL_WARN,"SUB: error allocating mem.\n");
+		   	continue;
+		}
+		op = ot;
 		if (iconv(icdsc, &ip, &ileft,
 			  &op, &oleft) == (size_t)(-1)) {
-			mp_msg(MSGT_SUBREADER,MSGL_WARN,"SUB: error recoding line (1).\n");
-			l++;
-			break;
+			mp_msg(MSGT_SUBREADER,MSGL_WARN,"SUB: error recoding line.\n");
+			free(ot);
+			continue;
 		}
-		if (!(ot = malloc(op - icbuffer + 1))){
-			mp_msg(MSGT_SUBREADER,MSGL_WARN,"SUB: error allocating mem.\n");
-			l++;
-		   	break;
-		}
 		*op='\0' ;
-		strcpy (ot, icbuffer);
 		free (sub->text[l]);
 		sub->text[l] = ot;
 	}
-	if (l){
-		for (l = sub->lines; l;)
-			free (sub->text[--l]);
-		return ERR;
-	}
 	return sub;
 }
-
-// for demux_ogg.c:
-subtitle* subcp_recode1 (subtitle *sub)
-{
-  int l=sub->lines;
-  size_t ileft, oleft;
-  
-  if(icdsc == (iconv_t)(-1)) return sub;
-
-  while (l){
-     char *ip = icbuffer;
-     char *op = sub->text[--l];
-     strlcpy(ip, op, ICBUFFSIZE);
-     ileft = strlen(ip);
-     oleft = ICBUFFSIZE - 1;
-		
-     if (iconv(icdsc, &ip, &ileft,
-	      &op, &oleft) == (size_t)(-1)) {
-	mp_msg(MSGT_SUBREADER,MSGL_V,"SUB: error recoding line (2).\n");
-	return sub;
-     }
-     *op='\0' ;
-  }
-  return sub;
-}
 #endif
 
 #ifdef USE_FRIBIDI
Index: libmpdemux/demux_ogg.c
===================================================================
--- libmpdemux/demux_ogg.c	(revision 20218)
+++ libmpdemux/demux_ogg.c	(working copy)
@@ -293,7 +293,7 @@
   mp_msg(MSGT_DEMUX,MSGL_DBG2,"Ogg sub lines: %d  first: '%s'\n",
       ogg_sub.lines, ogg_sub.text[0]);
 #ifdef USE_ICONV
-  subcp_recode1(&ogg_sub);
+  subcp_recode(&ogg_sub);
 #endif
   vo_sub = &ogg_sub;
   vo_osd_changed(OSDTYPE_SUBTITLE);