[MPlayer-dev-eng] [PATCH] Parsing simplification in vobsub

ubitux ubitux at gmail.com
Tue Apr 13 06:10:06 CEST 2010


On Tue, Apr 13, 2010 at 04:54:58AM +0200, ubitux wrote:
> Are you interested in that kind of patches? The attached patch shows a
> simpler way of parsing timestamp/filepos lines. I can also do it for
> parse_origin and parse_id, but I'd like to know if this can be accepted.
> 
> Also, I noted vobsub_parse_id allows an id with n characters, with
> mallocs/free, etc. But DVDs only allow 2 characters per language, it can't
> be more, so can I make a patch to simplify this while reducing parsing
> lines?
> 

Anyway, I just did it. So here is the patch. Also, I've typo fixed the
function I changed (like the line split about lang at the end of the
patch).

If this patch is accepted, I'd like to make another one, just about
typo and trivial cosmetics.

-- 
ubitux
-------------- next part --------------
Index: vobsub.c
===================================================================
--- vobsub.c	(revision 31035)
+++ vobsub.c	(working copy)
@@ -484,7 +484,7 @@
 } packet_t;
 
 typedef struct {
-    char *id;
+    char lang[3];
     packet_t *packets;
     unsigned int packets_reserve;
     unsigned int packets_size;
@@ -507,7 +507,7 @@
 
 static void packet_queue_construct(packet_queue_t *queue)
 {
-    queue->id = NULL;
+    strcpy(queue->lang, "xx");
     queue->packets = NULL;
     queue->packets_reserve = 0;
     queue->packets_size = 0;
@@ -622,28 +622,16 @@
     return 0;
 }
 
-static int vobsub_add_id(vobsub_t *vob, const char *id, size_t idlen,
-                         const unsigned int index)
+static int vobsub_add_id(vobsub_t *vob, const char *id, const unsigned int index)
 {
     if (vobsub_ensure_spu_stream(vob, index) < 0)
         return -1;
-    if (id && idlen) {
-        if (vob->spu_streams[index].id)
-            free(vob->spu_streams[index].id);
-        vob->spu_streams[index].id = malloc(idlen + 1);
-        if (vob->spu_streams[index].id == NULL) {
-            mp_msg(MSGT_VOBSUB, MSGL_FATAL, "vobsub_add_id: malloc failure");
-            return -1;
-        }
-        vob->spu_streams[index].id[idlen] = 0;
-        memcpy(vob->spu_streams[index].id, id, idlen);
-    }
+    strcpy(vob->spu_streams[index].lang, id);
     vob->spu_streams_current = index;
     mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_VOBSUB_ID=%d\n", index);
-    if (id && idlen)
-        mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_VSID_%d_LANG=%s\n", index, vob->spu_streams[index].id);
+    mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_VSID_%d_LANG=%s\n", index, vob->spu_streams[index].lang);
     mp_msg(MSGT_VOBSUB, MSGL_V, "[vobsub] subtitle (vobsubid): %d language %s\n",
-           index, vob->spu_streams[index].id);
+           index, vob->spu_streams[index].lang);
     return 0;
 }
 
@@ -665,101 +653,41 @@
     return -1;
 }
 
+/* Note on sscanf: spaces matter as they allow n whitespaces */
+
+/* id: xx, index: n */
 static int vobsub_parse_id(vobsub_t *vob, const char *line)
 {
-    // id: xx, index: n
-    size_t idlen;
-    const char *p, *q;
-    p  = line;
-    while (isspace(*p))
-        ++p;
-    q = p;
-    while (isalpha(*q))
-        ++q;
-    idlen = q - p;
-    if (idlen == 0)
+    char id[3];
+    unsigned int index;
+
+    if (sscanf(line, " %2s, index: %d", id, &index) != 2)
         return -1;
-    ++q;
-    while (isspace(*q))
-        ++q;
-    if (strncmp("index:", q, 6))
-        return -1;
-    q += 6;
-    while (isspace(*q))
-        ++q;
-    if (!isdigit(*q))
-        return -1;
-    return vobsub_add_id(vob, p, idlen, atoi(q));
+    return vobsub_add_id(vob, id, index);
 }
 
+/* timestamp: HH:MM:SS.mmm, filepos: 0nnnnnnnnn */
 static int vobsub_parse_timestamp(vobsub_t *vob, const char *line)
 {
-    // timestamp: HH:MM:SS.mmm, filepos: 0nnnnnnnnn
-    const char *p;
     int h, m, s, ms;
     off_t filepos;
-    while (isspace(*line))
-        ++line;
-    p = line;
-    while (isdigit(*p))
-        ++p;
-    if (p - line != 2)
+
+    if (sscanf(line, " %d:%d:%d:%d , filepos: %lx ", &h, &m, &s, &ms, &filepos) != 5)
         return -1;
-    h = atoi(line);
-    if (*p != ':')
-        return -1;
-    line = ++p;
-    while (isdigit(*p))
-        ++p;
-    if (p - line != 2)
-        return -1;
-    m = atoi(line);
-    if (*p != ':')
-        return -1;
-    line = ++p;
-    while (isdigit(*p))
-        ++p;
-    if (p - line != 2)
-        return -1;
-    s = atoi(line);
-    if (*p != ':')
-        return -1;
-    line = ++p;
-    while (isdigit(*p))
-        ++p;
-    if (p - line != 3)
-        return -1;
-    ms = atoi(line);
-    if (*p != ',')
-        return -1;
-    line = p + 1;
-    while (isspace(*line))
-        ++line;
-    if (strncmp("filepos:", line, 8))
-        return -1;
-    line += 8;
-    while (isspace(*line))
-        ++line;
-    if (! isxdigit(*line))
-        return -1;
-    filepos = strtol(line, NULL, 16);
     return vobsub_add_timestamp(vob, filepos, vob->delay + ms + 1000 * (s + 60 * (m + 60 * h)));
 }
 
+/* org: X, Y */
 static int vobsub_parse_origin(vobsub_t *vob, const char *line)
 {
-    // org: X,Y
-    char *p;
-    while (isspace(*line))
-        ++line;
-    if (!isdigit(*line))
-        return -1;
-    vob->origin_x = strtoul(line, &p, 10);
-    if (*p != ',')
-        return -1;
-    ++p;
-    vob->origin_y = strtoul(p, NULL, 10);
-    return 0;
+    unsigned int x, y;
+
+    if (sscanf(line, " %d , %d", &x, &y) == 2) {
+        vob->origin_x = x;
+        vob->origin_y = y;
+        return 0;
+    }
+    return -1;
 }
 
 unsigned int vobsub_palette_to_yuv(unsigned int pal)
@@ -1074,7 +1002,7 @@
 char *vobsub_get_id(void *vobhandle, unsigned int index)
 {
     vobsub_t *vob = (vobsub_t *) vobhandle;
-    return (index < vob->spu_streams_size) ? vob->spu_streams[index].id : NULL;
+    return (index < vob->spu_streams_size) ? vob->spu_streams[index].lang : NULL;
 }
 
 int vobsub_get_id_by_index(void *vobhandle, unsigned int index)
@@ -1106,19 +1034,21 @@
     return j;
 }
 
-int vobsub_set_from_lang(void *vobhandle, unsigned char * lang)
+int vobsub_set_from_lang(void *vobhandle, unsigned char *lang)
 {
     int i;
-    vobsub_t *vob= (vobsub_t *) vobhandle;
+    vobsub_t *vob = vobhandle;
+
     while (lang && strlen(lang) >= 2) {
         for (i = 0; i < vob->spu_streams_size; i++)
-            if (vob->spu_streams[i].id)
-                if ((strncmp(vob->spu_streams[i].id, lang, 2) == 0)) {
-                    vobsub_id = i;
-                    mp_msg(MSGT_VOBSUB, MSGL_INFO, "Selected VOBSUB language: %d language: %s\n", i, vob->spu_streams[i].id);
-                    return 0;
-                }
-        lang+=2;while (lang[0]==',' || lang[0]==' ') ++lang;
+            if ((strncmp(vob->spu_streams[i].lang, lang, 2) == 0)) {
+                vobsub_id = i;
+                mp_msg(MSGT_VOBSUB, MSGL_INFO, "Selected VOBSUB language: %d language: %s\n", i, vob->spu_streams[i].lang);
+                return 0;
+            }
+        lang += 2;
+        while (lang[0] == ',' || lang[0] == ' ')
+            lang++;
     }
     mp_msg(MSGT_VOBSUB, MSGL_WARN, "No matching VOBSUB language found!\n");
     return -1;


More information about the MPlayer-dev-eng mailing list