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 603471 - [flacdec] not timestamping output buffers
[flacdec] not timestamping output buffers
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
0.10.x
Other Linux
: Normal normal
: 0.10.18
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-12-01 12:18 UTC by Sebastian Dröge (slomo)
Modified: 2010-01-06 13:23 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Sebastian Dröge (slomo) 2009-12-01 12:18:04 UTC
Hi,
when creating an Ogg/FLAC file with oggmux like this:
gst-launch-0.10 audiotestsrc num-buffers=1000 ! flacenc ! oggmux ! filesink location=test-flac.ogg

Playback works fine but seeking is completely broken with latest GIT while in 0.10.25 it worked.
Comment 1 Wim Taymans 2009-12-09 15:17:36 UTC
flac code is not so nice anymore.. lots of stuff with granulepos and conversions between samples and time. I guess the oggdemux rewrite broke this fragile piece of code.
Comment 2 David Schleef 2009-12-09 20:22:35 UTC
It looks like flacdec isn't timestamping outgoing buffers here.
Comment 3 Tim-Philipp Müller 2009-12-10 01:02:00 UTC
There's a bit of a procedural problem here as well: either we release core, base and -good together (with a fixed flacdec), or the new oggdemux needs to work with the old flacdec as well.

I think the problem is just that flacdec assumes that only timestamps OR buffer end offsets are set, and assumes the absence/presence of one implies the presence/absence of the other. Removing the code that uses dec->cur_granulepos makes it work fine again.

Another issue/regression is that the new oggdemux doesn't handle the 'nonstandard' flac-in-ogg mapping properly (ie. does not detect it leading to errors).
Comment 4 David Schleef 2009-12-11 07:00:48 UTC
I just fixed the old-style fLaC.

It sounds like we can work around this by preventing oggdemux from putting timestamps on flac streams until the next release.  Not pretty, though.
Comment 5 Sebastian Dröge (slomo) 2009-12-11 08:05:05 UTC
(In reply to comment #4)
> I just fixed the old-style fLaC.
> 
> It sounds like we can work around this by preventing oggdemux from putting
> timestamps on flac streams until the next release.  Not pretty, though.


...and making flacdec work with and without timestamps for the next -good release? Can't we just get a new -good release done with just the flacdec change? :)

Also, does the same problem apply to Speex and CELT?
Comment 6 Sebastian Dröge (slomo) 2009-12-11 08:10:19 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > I just fixed the old-style fLaC.
> > 
> > It sounds like we can work around this by preventing oggdemux from putting
> > timestamps on flac streams until the next release.  Not pretty, though.
> 
> 
> ...and making flacdec work with and without timestamps for the next -good
> release? Can't we just get a new -good release done with just the flacdec
> change? :)
> 
> Also, does the same problem apply to Speex and CELT?

Probably, at least seeking is broken with CELT and the CELT elements are based on the Speex elements (at least the timestamp/granulepos handling).
Comment 7 Mark Nauwelaerts 2010-01-06 13:22:40 UTC
On the one hand, some testing suggests that flac ogg files (as produced by pipeline given above) do play and even allow for seeking (maybe due to intermediate oggdemux fixes?).

On the other hand, as mentioned earlier, current flacdec is not timestamping outgoing buffers.  That is basically a bug in flacdec (not so much a regression) since it does not seem to do so either in e.g. matroskademux case.

[apparently, it 'just works' because newsegment events do make it through, so basesink just renders based on that without further syncing]

The following commit should take care of the latter
(as mentioned above, flac code could do with some more cleanup, but will likely get to that a bit later on):

commit a76af918d0d2a4c391e84be525efb7834c75ad26
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Wed Jan 6 14:06:14 2010 +0100

    flacdec: really use upstream timestamp if there is one

    See/fixes #603471.