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 363119 - [Audiorate] rounding errors cause 'clicking' in sound
[Audiorate] rounding errors cause 'clicking' in sound
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-10-18 11:52 UTC by Edward Hervey
Modified: 2008-05-29 10:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed fix (6.25 KB, patch)
2006-10-18 11:52 UTC, Edward Hervey
accepted-commit_now Details | Review
Update for changes in audio rate (6.91 KB, patch)
2006-10-19 15:50 UTC, Edward Hervey
none Details | Review
Updated patch (13.11 KB, patch)
2006-10-26 15:50 UTC, Edward Hervey
none Details | Review

Description Edward Hervey 2006-10-18 11:52:33 UTC
Audiorate has rounding errors which lead to 'clicking' in outputted sound.

A simple audiotestsrc ! audiorate ! alsasink is enough to show that.

A solution to this problem is to:
1/ convert the expected offset into time
2/ calculate the difference between the incoming buffer and the expected time
3/ convert that difference to offset
4/ use the offset difference for all further calculation

This hides the rounding errors.
Comment 1 Edward Hervey 2006-10-18 11:52:59 UTC
Created attachment 74938 [details] [review]
Proposed fix
Comment 2 Edward Hervey 2006-10-19 15:50:25 UTC
Created attachment 75022 [details] [review]
Update for changes in audio rate

The rate might change, for which we need to recalculate the new ->next_offset.
Comment 3 Tim-Philipp Müller 2006-10-21 16:44:31 UTC
Added some fairly basic test cases:

  2006-10-21  Tim-Philipp Müller  <tim at centricular dot net>

        * tests/check/Makefile.am:
        * tests/check/elements/.cvsignore:
        * tests/check/elements/audiorate.c: (probe_cb), (got_buf),
        (do_perfect_stream_test), (GST_START_TEST), (audiorate_suite):
          Add some basic unit tests for audiorate. Disabled at the moment
          since it doesn't pass yet (see bug #363119).

Patch seems to work great to fix up the stream for the normal case, but there appear to be one or two corner-cases where it fails with an off-by-one offset when buffers get dropped (might be a bug in the unit test though, haven't really looked into it).

Btw, %lld and %llu aren't really portable in printf statements, better to use "%" G_GINT64_FORMAT  and "%" G_GUINT64_FORMAT.

Comment 4 Edward Hervey 2006-10-26 15:50:20 UTC
Created attachment 75450 [details] [review]
Updated patch

This patch is saner and goes through tim's unit test.
I put an explanation of how the modifications work as a comment above _chain().
Comment 5 Sebastian Dröge (slomo) 2007-03-02 08:34:01 UTC
Is there any progress on this bug and the last attached patch?
Comment 6 Tim-Philipp Müller 2007-05-04 11:40:17 UTC
What's with this patch? Is it still needed, or has this been fixed already by the various other audiorate commits?
Comment 7 Tim-Philipp Müller 2007-06-19 14:46:11 UTC
Ping #3
Comment 8 Edward Hervey 2007-07-24 17:22:31 UTC
unsure about the outcome of this. If the current code in cvs goes through tim's unit test... I'm fine with having this bug closed.
Comment 9 Tim-Philipp Müller 2007-07-26 22:58:24 UTC
> unsure about the outcome of this. If the current code in cvs goes through tim's
> unit test... I'm fine with having this bug closed.

The unit test only checks for formal correctness of the output. If the audio data is messed up in the process and clicks introduced by whatever, it won't recognise that.
Comment 10 Sebastian Dröge (slomo) 2008-05-29 10:03:07 UTC
Ok, there were a few changes by MikeS to audiorate since this patch was created and his changes were addressing similar issues. Also I can't produce any clicks with the latest code anymore so let's close this bug as obsolete now... if someone disagrees please reopen.