[FFmpeg-devel] [PATCH] avformat/rtpdec: Fix prft wallclock time.

[Alok Priyadarshi]

Ping. Could someone please review this patch. Thanks.

On Thu, Mar 25, 2021, 6:40 AM Alok Priyadarshi <alokpr at gmail.com> wrote:

> In effect the patch does replace that one line. But it also adds the steps
> to illustrate how the wallclock is calculated. Adding all the calculations
> in a single line will make it too long and hard to read.
>
> Note that delta_timestamp can be negative. It typically happens after rtcp
> SR is received and last_rtcp_ntp_time/last_rtcp_timestamp are refreshed.
> The packet timestamp can be less than last_rtcp_timestamp for a brief
> period of time. So it is necessary to explicitly cast both - timestamp
> and last_rtcp_timestamp - to int64 before calculating delta. This was
> another bug in the old code in addition to missing timebase scaling.
>
> On Thu, Mar 25, 2021 at 2:23 AM Carl Eugen Hoyos <ceffmpeg at gmail.com>
> wrote:
>
>> Am Do., 25. März 2021 um 05:47 Uhr schrieb Alok Priyadarshi <
>> alokpr at gmail.com>:
>> >
>> > Timestamp difference is available in media timebase (1/90K) where as
>> > rtcp time is in the default microseconds timebase. This patch fixes
>> > the calculated prft wallclock time by rescaling the timestamp delta
>> > to the microseconds timebase.
>> > ---
>> >  libavformat/rtpdec.c | 9 +++++++--
>> >  1 file changed, 7 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
>> > index b935dba1b8..21c1d01785 100644
>> > --- a/libavformat/rtpdec.c
>> > +++ b/libavformat/rtpdec.c
>> > @@ -585,14 +585,19 @@ void ff_rtp_parse_set_crypto(RTPDemuxContext *s,
>> const char *suite,
>> >  }
>> >
>> >  static int rtp_set_prft(RTPDemuxContext *s, AVPacket *pkt, uint32_t
>> timestamp) {
>> > +    int64_t rtcp_time, delta_timestamp, delta_time;
>> > +
>> >      AVProducerReferenceTime *prft =
>> >          (AVProducerReferenceTime *) av_packet_new_side_data(
>> >              pkt, AV_PKT_DATA_PRFT, sizeof(AVProducerReferenceTime));
>> >      if (!prft)
>> >          return AVERROR(ENOMEM);
>> >
>> > -    prft->wallclock = ff_parse_ntp_time(s->last_rtcp_ntp_time) -
>> NTP_OFFSET_US +
>>
>> > -                      timestamp - s->last_rtcp_timestamp;
>>
>> Wouldn't this patch get much more readable if you only replace this line?
>>
>> > +    rtcp_time = ff_parse_ntp_time(s->last_rtcp_ntp_time) -
>> NTP_OFFSET_US;
>> > +    delta_timestamp = (int64_t)timestamp -
>> (int64_t)s->last_rtcp_timestamp;
>> > +    delta_time = av_rescale_q(delta_timestamp, s->st->time_base,
>> AV_TIME_BASE_Q);
>> > +
>> > +    prft->wallclock = rtcp_time + delta_time;
>>
>> Carl Eugen
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
>