[gstreamer-bugs] [Bug 586570] Add GAP Flag support to audioresample

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=586570
  GStreamer | gst-plugins-base | git

--- Comment #18 from Stefan Kost (gstreamer, gtkdoc dev) <ensonic at sonicpulse.de> 2010-09-14 11:28:27 UTC ---
Review of attachment 170220:
 --> (https://bugzilla.gnome.org/review?bug=586570&attachment=170220)

Nice work! I added a few comments and it would be cool if you could change why
the tests fail.
Also, please also drop the "common" change from the patch.

::: gst/audioresample/gstaudioresample.c
@@ +790,3 @@
+  guint out_len, out_processed;
+  guint num, den;
+  void* buf;

s/void*/gpointer/ or just use "guint8 *" as this is what the actual functions
take

@@ +806,3 @@
+  resample->funcs->process (resample->state, NULL, &in_processed, buf,
&out_processed);
+  g_free(buf);
+

wouldn't you need to zero buf? And if so, wouldn'ät it be easier to support
passing NULL and make process writing zeros in that case?

@@ +807,3 @@
+  g_free(buf);
+
+  g_assert(in_len == in_processed);

that makes no sense here, as you just set this above.

::: gst/audioresample/gstaudioresample.h
@@ +67,3 @@
+  guint count_gap;
+  guint count_nongap;
+

these are not really counting gap/nongap, right? What about

guint num_gap_samples;
guint num_{non_gap,reg,norm}_samples;
(not sure about the 2nd)

::: gst/audioresample/speex_resampler.h
@@ +338,3 @@
+ * @param st Resampler state
+ */
+int speex_resampler_get_filt_len(SpeexResamplerState *st);

please use normal /* */ comments unless it is really a gtk-doc style formatted
comment. The above one will cause a gtk-doc warning.

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