[gstreamer-bugs] [Bug 169936] [PATCH] [subparse] support for SAMI subtitles

[GStreamer (bugzilla.gnome.org)]

Do not reply to this via email (we are currently unable to handle email
responses and they get discarded).  You can add comments to this bug at
http://bugzilla.gnome.org/show_bug.cgi?id=169936
 GStreamer | gst-plugins-base | Ver: HEAD CVS


Andy Wingo changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #64213|none                        |needs-work
               Flag|                            |




------- Comment #21 from Andy Wingo  2006-04-25 10:07 UTC -------
OK at this point just a few nitpicks:

1) Avoid declaring variables in the middle of a scope, e.g. has_italic in
pop_state(), value in start_sync -- we still compile on sun's forte

2) The idiom of iterating over attributes would be more clearly written as:
    for (i = 0; (atts[i] != NULL); i+=2) {
      const xmlChar *key, *value;
      key = atts[i];
      value = atts[i+1];

3) According to
http://www.jamesh.id.au/articles/libxml-sax/libxml-sax.html#characters, the
string passed to characters_sami is not guaranteed to be null-terminated; you
need to use the g_string_append_len.

4) When configuring GStreamer without XML support, the sami parser shouldn't be
built because there is no libxml. Please surround the sami code in #ifndef
GST_DISABLE_LOADSAVE_REGISTRY.

Other than that, looking very good. This will be nice to have :-)


-- 
Configure bugmail: http://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.