GNOME Bugzilla – Bug 761943
gstrtpbasepayload: set DEFAULT_PERFECT_RTPTIME to FALSE
Last modified: 2018-11-03 11:44:46 UTC
Created attachment 320983 [details] [review] patch A very common pipeline is feeding an encoder inheriting from GstAudioEncoder into a payloader inheriting from GstRtpBasePayload. If this is the case, the rtp-timestamp will, if perfect_rtptime is TRUE, reflect the amount of bytes in each packet, and not (like it should) reflect the timestamp of the packets in the clock-rate domain. Example: 20ms AAC packets with clock-rate of 90000, should have the rtp-timestamp incrementing with 90000 * 20 / 1000 = 1800. Currently, because perfect_rtptime is default TRUE, the AAC-packets will be incrementing with the buffer-size of the packets, which is typically 64000 * 20 / 1000 = 1280. In the case that bitrate and clock-rate are the same (like with ALAW and MULAW) it will work by chance, which is probably why it has not been discovered earlier.
Wim, any opinion? perfect-rtptime=TRUE seems wrong to me as default.
Increasing by the buffer size is a bug here.. it should increase by the number of samples.. This sounds like a bug in the payloader.
Created attachment 327926 [details] [review] rtpmpeg4pay: Don't produce timestamps based on byte count Perfect RTP time is only used for payloaders that support it, they do that by giving the sample count in GST_BUFFER_OFFSET, rtpmp4gpay was just putting the byte count in there, explaining the effect your seeing! The GST_BUFFER_OFFSET of output buffers returned to GstRtpBasePayload should reflect the number of "samples" in the unit of the RTP clock in this buffer. If this is not true, then it shouldn't be set.
Created attachment 327927 [details] [review] rtpmp4gpay: Don't produce timestamps based on byte count The GST_BUFFER_OFFSET of output buffers returned to GstRtpBasePayload should reflect the number of "samples" in the unit of the RTP clock in this buffer. If this is not true, then it shouldn't be set.
Fix committed to rtpmp4gpay, I think all of the other payloaders have the right behaviour. commit e21cf3bc1c85ff7ad235c36a99ee169784040008 Author: Olivier Crête <olivier.crete@collabora.com> Date: Sun May 15 12:16:23 2016 +0200 rtpmp4gpay: Don't produce timestamps based on byte count The GST_BUFFER_OFFSET of output buffers returned to GstRtpBasePayload should reflect the number of "samples" in the unit of the RTP clock in this buffer. If this is not true, then it shouldn't be set. https://bugzilla.gnome.org/show_bug.cgi?id=761943
The reported issue did not involve rtpmp4gpay in any shape or form, but the fact the "perfect-rtptime" does not consider the clock-rate at all, making it completely useless IMHO. My point in the initial description was that this works "by accident" for any payloadtype that happens to have clock-rate == samplerate, but the whole concept is flawed and should be removed. (IMHO) :)
For most audio codecs, it is explicitly defined that clock-rate=sample-rate, for those where it isn't, there is sometimes a direct mapping that can be obtained. Do you have any pipeline where it produces the wrong result? In theory, the code should do nothing if the subclass doesn't set GST_BUFFET_OFFSET() on the output buffer and it only does that on fixed bitrate codecs right now (those that use GstRtpBaseAudioPayload).
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/252.