[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 #64141|none                        |needs-work
               Flag|                            |




------- Comment #19 from Andy Wingo  2006-04-24 09:36 UTC -------
0) You are awesome!

1) Your patch should use gstreamer coding style; you can guarantee this by
running the gst-indent script, from gstreamer/tools/, on your file before
making the diff.

2) You should not be using statically-linked data, it prevents two instances of
the plugin from being run at once; you should put that data into the
ParserState->user_data.

3) Consider using GString for accumulating data, it makes the code clearer

4) You don't do any error handling; what happens when you feed it a malformed
subtitle?

Thanks!


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