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 707230 - flacparse: disregards container timestamps
flacparse: disregards container timestamps
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal major
: 1.2.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-09-01 15:43 UTC by Matej Knopp
Modified: 2013-11-26 11:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Description Matej Knopp 2013-09-01 15:43:13 UTC 
flacparse seems to always set absolute timestamps from sample number, even if the buffer is already timestamped.

Also could someone explain this code?

    GST_BUFFER_OFFSET (buffer) =
        gst_util_uint64_scale (GST_BUFFER_OFFSET_END (buffer), GST_SECOND,
        flacparse->samplerate);
    GST_BUFFER_DURATION (buffer) =
        GST_BUFFER_OFFSET (buffer) - GST_BUFFER_TIMESTAMP (buffer);
Comment 1 Sebastian Dröge (slomo) 2013-09-02 10:10:43 UTC 
Yes that indeed looks wrong. It should only do these calculations if upstream does not provide timestamps (or does not send a segment in TIME format), otherwise it should use the upstream timestamps and optionally interpolate between them.

Want to provide a patch?

The OFFSET/OFFSET_END calculation is there for the Ogg granulepos :) It's unfortunately historically stored in these fields.
Comment 2 Sebastian Dröge (slomo) 2013-09-02 10:11:33 UTC 
It's wrong in wavpackparse too
Comment 3 Matej Knopp 2013-09-02 10:14:11 UTC 
I have some other things to fix first, after that I can look at this. 
I think this might be little more complicated than just a check for upstream timestamps, because if there are only some upstream timestamps the parser should probably fill the empty ones, but relative to provided timestamps, right?
Comment 4 Tim-Philipp Müller 2013-09-02 10:26:26 UTC 
You would think baseparse handles this already..
Comment 5 Matej Knopp 2013-09-02 10:32:07 UTC 
Does it? If so what does it need, is it enough to set valid duration on buffer?
Comment 6 Sebastian Dröge (slomo) 2013-09-02 11:36:14 UTC 
The code seems to do something like that with the duration, yes.
Comment 7 Mark Nauwelaerts 2013-11-16 14:23:49 UTC 
Following commit should have baseparse enforce some nice behaviour wrt upstream timestamps:

commit e24722c52c5840fdf9e5825ba624d2e0aa129494
Author: Mark Nauwelaerts <mnauw@users.sourceforge.net>
Date:   Sat Nov 16 15:17:57 2013 +0100

    baseparse: ensure to preserve upstream timestamps
    
    ... rather than have subclass coming up with an internally parsed one.
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=707230