[gst-devel] Re: [gst-cvs] fcarvalho gst-plugins-bad: gst-plugins-bad/ gst-plugins-bad/gst/speed/

[Tim Müller]

On Thu, 2005-09-08 at 08:56 -0700, Flavio Carvalho de Oliveira wrote:

Hi Flavio,

> CVS Root:       /cvs/gstreamer
> Module:         gst-plugins-bad
> Changes by:     fcarvalho
> Date:           Thu Sep 08 2005  08:56:50 PDT
> 
> Log message:
> Ported speed Plugin to GStreamer 0.9
> 
> Modified files:
>     .               : ChangeLog PORTED_09 configure.ac
>     gst/speed       : Makefile.am gstspeed.c
> 
> Links:
> http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gst-plugins-bad/ChangeLog.diff?r1=1.1830&r2=1.1831
> http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gst-plugins-bad/PORTED_09.diff?r1=1.9&r2=1.10
> http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gst-plugins-bad/configure.ac.diff?r1=1.590&r2=1.591
> http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gst-plugins-bad/gst/speed/Makefile.am.diff?r1=1.12&r2=1.13
> http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gst-plugins-bad/gst/speed/gstspeed.c.diff?r1=1.37&r2=1.38
> 

please don't get me wrong, but I think it might be a good idea if you
submitted your ported plugins as patches into bugzilla for review for
the time being instead of comitting them directly to CVS.

Some quick comments about the gstspeed port:
  - you didn't port the query function and just implement one that
returns FALSE to all queries
  - the chain function has an if (GST_IS_EVENT(buffer)) block which
doesn't belong there in 0.9; and the event handling that is there, in
particular the newsegment stuff, has just been commented out.
  - gst_speed_set_caps() has a boolean return type, but you return
GST_PAD_LINK_REFUSED (=-6) in the case of error instead of FALSE
  - the buffer-frames stuff has been removed in 0.9 (see Andy Wingo's
revent post to the gst-devel mailing list)
  - the chain function leaks the input buffer

Have you seen the 0.9 porting guide in the appendix of the Plugin
Writer's Guide and read the 0.9 PWG? If you discover anything that's
missing there or should be explained in more detail, please do point it
out.

Cheers
 -Tim