Ping. Could someone please review this patch. Thanks.
On Thu, Mar 25, 2021, 6:40 AM Alok Priyadarshi <alo...@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 <ceffm...@gmail.com>
> wrote:
>
>> Am Do., 25. März 2021 um 05:47 Uhr schrieb Alok Priyadarshi <
>> alo...@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@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
