[gstreamer-bugs] [Bug 354451] [mythtvsrc] Plugin for MythTV source

[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=354451

  GStreamer | gst-plugins-bad | Ver: HEAD CVS


Tim-Philipp Müller changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #79898|none                        |committed
               Flag|                            |
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|HEAD                        |0.10.5




------- Comment #16 from Tim-Philipp Müller  2007-01-13 12:37 UTC -------
Looks like this was committed, so closing this bug:

 2007-01-12  Edgard Lima <edgard.lima at indt.org.br>

        Patch by: Rosfran Borges <rosfran.borges at indt.org.br>

        * configure.ac:
        * gst-plugins-bad.spec.in:
        * ext/Makefile.am:
        * ext/mythtv/Makefile.am:
        * ext/mythtv/gstmythtvsrc.c:
        * ext/mythtv/gstmythtvsrc.h:
        MythTV client source plugin created.

Just some quick comments from glancing over the code:

 - your configure.ac patch removes bits from configure.ac
   that it shouldn't be touching, like the tests/ directories;
   the committed version is ok though.

 - in the _set_property() function you do goto done; with the
   object lock held in case of an error, but the done label is
   after where the object is unlocked again - that will deadlock.

 - there is no need to use things like
      GST_INFO ("[%s] this and that", __FUNCTION__);
   the function name will always be printed automatically
   in the GStreamer debug log. Also, one exclamation mark
   is usually enough at the end of a debug statement :)

 - there are g_usleep (500); and sleep(9); statements and
   similar in the code in the _start() function this is almost
   always wrong and a sign that things should be designed
   differently. Especially a sleep of 9 seconds is outright
   evil, since it can't be interrupted, and an application
   trying to interrupt it would block for at least that long
   with the interface freezing up etc. Same for the normal
   start-up case where this is triggered (arguably that's
   a design problem in GstBaseSrc though, since it also
   applies when doing open() on CD/DVD devices and the like).


-- 
Configure bugmail: http://bugzilla.gnome.org/userprefs.cgi?tab=email