After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 703901 - audioecho: Output broken if delay value got changed when playing or paused
audioecho: Output broken if delay value got changed when playing or paused
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.0.5
Other Linux
: Normal normal
: 1.1.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-07-09 19:41 UTC by SuperCat
Modified: 2013-07-12 07:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
audioecho plug-in patch (790 bytes, patch)
2013-07-09 19:41 UTC, SuperCat
needs-work Details | Review
Modified patch of audioecho plug-in (1.35 KB, patch)
2013-07-10 13:48 UTC, SuperCat
needs-work Details | Review
Patch created by git (1.95 KB, patch)
2013-07-11 19:30 UTC, SuperCat
committed Details | Review

Description SuperCat 2013-07-09 19:41:56 UTC
Created attachment 248773 [details] [review]
audioecho plug-in patch

Hello,

I have suffered this bug for a long time just since I started using GStreamer 0.10: If I changed "delay" property at the state of playing or paused, the output audio became noise and then got no output at all.

I have done some research on the audioecho plug-in and then write a patch for it (In the attachment).
Comment 1 Sebastian Dröge (slomo) 2013-07-10 07:30:01 UTC
Review of attachment 248773 [details] [review]:

::: audioecho.c.old
@@ +204,3 @@
       }
+      
+      //Reset the buffer and update delay frame number if delay value has changed.

Please use normal C comments /* */

@@ +206,3 @@
+      //Reset the buffer and update delay frame number if delay value has changed.
+      rate = GST_AUDIO_FILTER_RATE (self);
+      self->delay_frames = MAX (gst_util_uint64_scale (self->delay, rate, GST_SECOND), 1);

You should check that the delay is smaller/equal to max-delay. Otherwise the buffer will be too small and it will crash

@@ +207,3 @@
+      rate = GST_AUDIO_FILTER_RATE (self);
+      self->delay_frames = MAX (gst_util_uint64_scale (self->delay, rate, GST_SECOND), 1);
+      self->buffer_pos = 0; 

Resetting the buffer position should cause a short distortion whenever you set a different delay. Why is that necessary?
Comment 2 SuperCat 2013-07-10 13:48:28 UTC
Created attachment 248835 [details] [review]
Modified patch of audioecho plug-in

I have removed the comment and resized the buffer if max_delay has enlarged.
Here is my new patch.
Comment 3 Sebastian Dröge (slomo) 2013-07-11 08:09:34 UTC
Review of attachment 248835 [details] [review]:

Please attach this as a "git format-patch" style patch. For this commit the changes locally (check that git uses your mail address and name) and use "git format-patch -1"

::: audioecho.c.old
@@ +203,3 @@
         self->delay = delay;
         self->max_delay = MAX (delay, max_delay);
+        if(self->max_delay>max_delay) {

This happens only in READY state and there we don't have a buffer allocated yet. The code in line 195 is for the case when we're PAUSED/PLAYING already and have a buffer.
Comment 4 SuperCat 2013-07-11 19:30:54 UTC
Created attachment 248962 [details] [review]
Patch created by git

OK, I create the new one by git.