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 613206 - [rtph264depay] wrong timestamp for gst_base_rtp_depayload_push_ts?
[rtph264depay] wrong timestamp for gst_base_rtp_depayload_push_ts?
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
0.10.21
Other All
: Normal normal
: 0.10.22
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-03-18 00:46 UTC by dxssx
Modified: 2010-03-23 11:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
a patch for this (537 bytes, patch)
2010-03-18 00:46 UTC, dxssx
none Details | Review
patch for rtph264depay (634 bytes, patch)
2010-03-18 00:50 UTC, dxssx
none Details | Review
updated patch (1.15 KB, patch)
2010-03-18 01:12 UTC, dxssx
rejected Details | Review

Description dxssx 2010-03-18 00:46:37 UTC
Created attachment 156419 [details] [review]
a patch for this

Current code use the following code to generate a timestamp for gst_base_rtp_depayload_push_ts:
ts = GST_BUFFER_TIMESTAMP (buf);
ts = gst_segment_to_running_time (&depayload->segment, GST_FORMAT_TIME, ts);

But I think it should be rtptime like below:
ts = gst_rtp_buffer_get_timestamp (buf);

Although the timestamp will never be used in current gstreamer, this bug may be a problem in future.
Comment 1 dxssx 2010-03-18 00:50:01 UTC
Created attachment 156421 [details] [review]
patch for rtph264depay
Comment 2 dxssx 2010-03-18 01:12:55 UTC
Created attachment 156424 [details] [review]
updated patch
Comment 3 Mark Nauwelaerts 2010-03-18 10:03:20 UTC
AFAIK, rtph264depay is handling timestamps as it should, i.e. the same way the base depayloader does, which is not to rely on the rtptime.  The latter is taken into consideration by rtpjitterbuffer (if involved), see also e.g. README in -good gst/rtp.
Comment 4 dxssx 2010-03-18 10:37:55 UTC
Base depayloader calls gst_base_rtp_depayload_push_ts with rtptime not gstreamer runtime timestamp what rtph264depay does.

Althrough base depayloader make no use of rtptime in gst_base_rtp_depayload_push_ts now, 
rtph264depay should not pass gstreamer timestamp to gst_base_rtp_depayload_push_ts.
It may cause problem in future if base depayloader makes change of gst_base_rtp_depayload_push_ts.
Comment 5 Mark Nauwelaerts 2010-03-18 11:47:23 UTC
Ah, in that way.  Then I applied following fix, which does not request timestamping at all (as the provided buffer has already been timestamped):

commit fd5164af96f2ff80d92331422cd4892850913569
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Thu Mar 18 12:20:17 2010 +0100

    rtph264depay: do not call _push_ts with unneeded (and wrong) time parameter

    Fixes #613206.
Comment 6 dxssx 2010-03-19 00:02:18 UTC
ok.