[gstreamer-bugs] [Bug 349207] [PLUGIN-ADD] audiopanorama

[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=349207
 GStreamer | gst-plugins-good | Ver: HEAD CVS





------- Comment #6 from Tim-Philipp Müller  2006-08-18 09:55 UTC -------
Right, so let's move this on a bit:

First the nitpicking (minor things only):

 - Makefile.am needs GST_CONTROLLER_CFLAGS and more importantly
   GST_CONTROLLER_LIBS (or whatever they are called)

 - in gst_audio_panorama_get_unit_size() the g_return_is_fail() is
   unnecessary IMHO, I'd either make it an assert or remove it
   completely, same for the 'ret' check there

 - gst_audio_panorama_set_caps: should probably set process function
   to NULL on error true (could probably be written easier to just
   return (filter->process_func != NULL) at the end. Also, goto
   label 'Error' should not be capitalised.


I can't comment on the algorithm, but that's not really required anyway.

Everything looks good to me otherwise:

  - unit test is there,
  - base class is used,
  - documentation is provided
  - you are willing to maintain it
  - no license issues
  - I've looked over the code and it's fine and conforms to our style

so all the formal requirements seem to have been met.


One other thing I was wondering is whether the plugin should really be called
'audiopanorama' or if we might want to plan a bit ahead and anticipate the
addition of other 500-lines-of-code simple audio effects elements, which surely
don't need to get their own plugin/.so. Dunno, just a thought.

Oh, and float support would be nice too at some point ;)


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