[FFmpeg-devel] [PATCH] Calculate RTCP SR timestamps from packet timestamps

[Luca Abeni]

Hi Martin,

On 28/03/10 13:28, Martin Storsj? wrote:
> On Sun, 28 Mar 2010, Luca Abeni wrote:
>
>> On Sun, 2010-03-28 at 13:12 +0300, Martin Storsj? wrote:
>>> Currently, the timestamps for RTCP sender reports are calculated based on
>>> the system realtime clock. Since the start time for the RTP muxer can be
>>> set through the start_time_realtime field, this may be slightly (or quite
>>> much if the user explicitly wants that) different from the actual current
>>> time. This leads to sending RTCP SR packets where the RTP timestamp is way
>>> off from the RTP packets at the same time.
>>
>> In my understanding of the RFC, the only important thing is that the RTP
>> timestamp contained in the RTCP SR packet "matches" the NTP time
>> contained in the same RTCP packet. Isn't this correct?
>
> Yes, that's my understanding too. But nevertheless, it feels weird to send
> an RTCP packet syncing the NTP time to RTP timestamps for a RTP timestamp
> very far off from the current RTP packets.

Ok, this time I actually looked at the RFC :)
Section "6.4.1 SR: Sender Report RTCP Packet" of RFC 3550 says:
NTP timestamp: 64 bits
       Indicates the wallclock time (see Section 4) when this report was
       sent so that it may be used in combination with timestamps
       returned in reception reports from other receivers to measure
       round-trip propagation to those receivers.  Receivers should
       expect that the measurement accuracy of the timestamp may be
       limited to far less than the resolution of the NTP timestamp.
[...]
So, I misremembered: the NTP time _must_ indicate the time when the RTCP 
packet was sent, and not the nearest RTP timestamp from an RTP packet...
So, I think the current implementation is correct (and I now remember 
having problems in computing the RTT when the RTP muxer implemented a 
different behaviour in this regard).

Moreover, the RFC also says:
RTP timestamp: 32 bits
       Corresponds to the same time as the NTP timestamp (above), but in
       the same units and with the same random offset as the RTP
       timestamps in data packets.  This correspondence may be used for
       intra- and inter-media synchronization for sources whose NTP
       timestamps are synchronized, and may be used by media-independent
       receivers to estimate the nominal RTP clock frequency.  Note that
       in most cases this timestamp will not be equal to the RTP
       timestamp in any adjacent data packet.  Rather, it MUST be
       calculated from the corresponding NTP timestamp using the
       relationship between the RTP timestamp counter and real time as
       maintained by periodically checking the wallclock time at a
       sampling instant.

So, it seems to me that the RFC explicitly say that there will be RTCP 
SR packets where the RTP timestamp is way  off from the RTP packets at 
the same time.

>> I fear your patch can create problems when the application does not send
>> the frames "regularly", but buffers some of them to perform bandwidth
>> smoothing.
>
> Even in this case, the packets should be sent almost regularly, since it
> still is realtime transport protocol...

Packets will probably be sent almost regularly, but frames will probably 
not (consider VBR video).


>> Have you seen some problems with the current behaviour? Can you describe
>> them? (maybe some client is making assumptions about some relationship
>> between RTP timestamps and RTCP timestamps?)
>
> No, I haven't seen any concrete problems, but I felt it would be more
> consistent this way.
>
> The intent and idea is to contain all realtime clock usage to initializing
> first_rtcp_ntp_time - that way, the user can set that (via
> start_time_realtime) to whatever start time he wants, and every NTP
> timestamp in the session is sent relative to that.

It seems to me that the RFC explicitly requires to use the wallclock 
time, and using something different will create problems (in this case, 
when trying to computing the RTT based on RTCP RR packets).

If we can find a different solution which is still compliant with the 
RFC and does not break RTT computation, I am fine with it... But it 
seems to me that your patch explicitly goes against the RFC.


			Thanks,
				Luca