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 661983 - Regression: Reverse playback does not work for vorbis
Regression: Reverse playback does not work for vorbis
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal blocker
: 0.10.36
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-10-17 11:18 UTC by Wim Taymans
Modified: 2011-11-14 11:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
audiodecoder: improve reverse playback (7.34 KB, patch)
2011-10-19 14:32 UTC, Mark Nauwelaerts
committed Details | Review

Description Wim Taymans 2011-10-17 11:18:51 UTC
After vorbisdec was ported to audiodecoder, it does not handle reverse playback correctly. Audio sounds like stuttering.
Comment 1 Mark Nauwelaerts 2011-10-17 20:01:04 UTC
In what particular scenario is this (e.g. ogg with some vorbis) ?

Reverse playback in audiodecoder is pretty verbatim taken from vorbisdec, so all the data packet queuing and manipulation should amount to the same (unless some other pair of eyes can spot a difference).

Also, trying with some vorbis-from-matroska turned out timestamps that looked pretty decent (and continuous within some precision).
Comment 2 Wim Taymans 2011-10-18 10:22:40 UTC
I tested in 0.10 with seek.c on various ogg files with vorbis audio (also tested with gravity ogg/theora+vorbis).

This commit broke it:

f63f09483f22fd7e2672d3df58be8d8a69b4603c

I'll try to get some before and after timestamps/data dumps.
Comment 3 Wim Taymans 2011-10-19 08:17:23 UTC
It seems the audiodecoder base class does not correctly calculate the outgoing timestamps. I would have expected the audiosink to do a better job of interpolating them, though.

What should happen is that the clipping and timestamp interpolation should be done right before pushing the buffer. Where it is done now in the baseclass is correct for forward playback but wrong for reverse playback (it clips/interpolates and then puts the buffer in the output queue).

I first want to have a look at how the sink can be improved and then try to fix the base class.
Comment 4 Mark Nauwelaerts 2011-10-19 08:24:49 UTC
Indeed, audiodecoder could probably do with another round of timestamping (interpolating) as it queues decoded frames (or just before pushing those finally downstream), along with some clipping.
Comment 5 Mark Nauwelaerts 2011-10-19 14:32:09 UTC
Created attachment 199440 [details] [review]
audiodecoder: improve reverse playback

This should improve/fix reverse playback, as far as I can see from timestamp dumps, and hopefully also according to hearing ...
Comment 6 Mark Nauwelaerts 2011-11-14 11:10:18 UTC
As stated earlier, this implements as in Comment #3 and afaics should take care of this.  Feel free to re-open if otherwise protestable. 

commit 38615abdd8fa3f3c9b918b5a178a092182f835dd
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Wed Oct 19 16:30:27 2011 +0200

    audiodecoder: improve reverse playback
    
    ... by doing some more (reverse) timestamp interpolating and
    refactoring downstream pushing.
    
    Fixes #661983.