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

[Alok Priyadarshi]

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".