* [FFmpeg-devel] [PATCH v2] rtpdec: Fix RTP timestamp wraparound in Producer Reference Time @ 2025-06-07 8:06 Clément Péron 2025-06-07 23:33 ` Michael Niedermayer 2025-06-08 14:53 ` Kieran Kunhya via ffmpeg-devel 0 siblings, 2 replies; 6+ messages in thread From: Clément Péron @ 2025-06-07 8:06 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Clément Péron The rtp_set_prft() function incorrectly calculates the timestamp delta when RTP timestamps wrap around the 32-bit boundary. The current code: delta_timestamp = (int64_t)timestamp - (int64_t)s->last_rtcp_timestamp; treats both timestamps as large positive values, causing wraparound to produce a large negative delta instead of the correct small positive delta. For example, with a 90kHz video clock: - last_rtcp_timestamp = 0xFFFFFF00 (near wraparound) - timestamp = 0x00000100 (after wraparound) - Current result: delta ≈ -4.3 billion ticks ≈ -47,721 seconds - Expected result: delta ≈ +512 ticks ≈ +0.006 seconds This causes prft->wallclock to jump backward by approximately: - 90kHz video: ~47,721 seconds (~13.25 hours) - 48kHz audio: ~89,478 seconds (~24.9 hours) - 8kHz audio: ~536,871 seconds (~6.2 days) Fix by casting the subtraction result to int32_t, which correctly handles wraparound through modular arithmetic: delta_timestamp = (int32_t)(timestamp - s->last_rtcp_timestamp); This ensures the delta is always in the range [-2^31, 2^31-1], making wraparound produce the correct small positive values. Fixes timing jumps in applications that rely on Producer Reference Time for media synchronization. Signed-off-by: Clément Péron <peron.clem@gmail.com> --- libavformat/rtpdec.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c index 729bf83685..7664ab58bd 100644 --- a/libavformat/rtpdec.c +++ b/libavformat/rtpdec.c @@ -633,7 +633,8 @@ 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; + int64_t rtcp_time, delta_time; + int32_t delta_timestamp; AVProducerReferenceTime *prft = (AVProducerReferenceTime *) av_packet_new_side_data( @@ -642,7 +643,7 @@ static int rtp_set_prft(RTPDemuxContext *s, AVPacket *pkt, uint32_t timestamp) { return AVERROR(ENOMEM); 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_timestamp = (int32_t)(timestamp - 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; -- 2.48.1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] rtpdec: Fix RTP timestamp wraparound in Producer Reference Time 2025-06-07 8:06 [FFmpeg-devel] [PATCH v2] rtpdec: Fix RTP timestamp wraparound in Producer Reference Time Clément Péron @ 2025-06-07 23:33 ` Michael Niedermayer 2025-06-08 7:27 ` Clément Péron 2025-06-08 14:53 ` Kieran Kunhya via ffmpeg-devel 1 sibling, 1 reply; 6+ messages in thread From: Michael Niedermayer @ 2025-06-07 23:33 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 2869 bytes --] Hi On Sat, Jun 07, 2025 at 10:06:19AM +0200, Clément Péron wrote: > The rtp_set_prft() function incorrectly calculates the timestamp delta > when RTP timestamps wrap around the 32-bit boundary. The current code: > > delta_timestamp = (int64_t)timestamp - (int64_t)s->last_rtcp_timestamp; > > treats both timestamps as large positive values, causing wraparound to > produce a large negative delta instead of the correct small positive delta. > > For example, with a 90kHz video clock: > - last_rtcp_timestamp = 0xFFFFFF00 (near wraparound) > - timestamp = 0x00000100 (after wraparound) > - Current result: delta ≈ -4.3 billion ticks ≈ -47,721 seconds > - Expected result: delta ≈ +512 ticks ≈ +0.006 seconds > > This causes prft->wallclock to jump backward by approximately: > - 90kHz video: ~47,721 seconds (~13.25 hours) > - 48kHz audio: ~89,478 seconds (~24.9 hours) > - 8kHz audio: ~536,871 seconds (~6.2 days) > > Fix by casting the subtraction result to int32_t, which correctly > handles wraparound through modular arithmetic: > > delta_timestamp = (int32_t)(timestamp - s->last_rtcp_timestamp); > > This ensures the delta is always in the range [-2^31, 2^31-1], making > wraparound produce the correct small positive values. > > Fixes timing jumps in applications that rely on Producer Reference Time > for media synchronization. > > Signed-off-by: Clément Péron <peron.clem@gmail.com> > --- > libavformat/rtpdec.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c > index 729bf83685..7664ab58bd 100644 > --- a/libavformat/rtpdec.c > +++ b/libavformat/rtpdec.c > @@ -633,7 +633,8 @@ 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; > + int64_t rtcp_time, delta_time; > + int32_t delta_timestamp; > > AVProducerReferenceTime *prft = > (AVProducerReferenceTime *) av_packet_new_side_data( > @@ -642,7 +643,7 @@ static int rtp_set_prft(RTPDemuxContext *s, AVPacket *pkt, uint32_t timestamp) { > return AVERROR(ENOMEM); > > 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_timestamp = (int32_t)(timestamp - 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; will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Asymptotically faster algorithms should always be preferred if you have asymptotical amounts of data [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] [-- Attachment #2: Type: text/plain, Size: 251 bytes --] _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] rtpdec: Fix RTP timestamp wraparound in Producer Reference Time 2025-06-07 23:33 ` Michael Niedermayer @ 2025-06-08 7:27 ` Clément Péron 2025-06-08 20:09 ` Michael Niedermayer 0 siblings, 1 reply; 6+ messages in thread From: Clément Péron @ 2025-06-08 7:27 UTC (permalink / raw) To: FFmpeg development discussions and patches Hi, On Sun, 8 Jun 2025 at 01:33, Michael Niedermayer <michael@niedermayer.cc> wrote: > > Hi > > On Sat, Jun 07, 2025 at 10:06:19AM +0200, Clément Péron wrote: > > The rtp_set_prft() function incorrectly calculates the timestamp delta > > when RTP timestamps wrap around the 32-bit boundary. The current code: > > > > delta_timestamp = (int64_t)timestamp - (int64_t)s->last_rtcp_timestamp; > > > > treats both timestamps as large positive values, causing wraparound to > > produce a large negative delta instead of the correct small positive delta. > > > > For example, with a 90kHz video clock: > > - last_rtcp_timestamp = 0xFFFFFF00 (near wraparound) > > - timestamp = 0x00000100 (after wraparound) > > - Current result: delta ≈ -4.3 billion ticks ≈ -47,721 seconds > > - Expected result: delta ≈ +512 ticks ≈ +0.006 seconds > > > > This causes prft->wallclock to jump backward by approximately: > > - 90kHz video: ~47,721 seconds (~13.25 hours) > > - 48kHz audio: ~89,478 seconds (~24.9 hours) > > - 8kHz audio: ~536,871 seconds (~6.2 days) > > > > Fix by casting the subtraction result to int32_t, which correctly > > handles wraparound through modular arithmetic: > > > > delta_timestamp = (int32_t)(timestamp - s->last_rtcp_timestamp); > > > > This ensures the delta is always in the range [-2^31, 2^31-1], making > > wraparound produce the correct small positive values. > > > > Fixes timing jumps in applications that rely on Producer Reference Time > > for media synchronization. > > > > Signed-off-by: Clément Péron <peron.clem@gmail.com> > > --- > > libavformat/rtpdec.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c > > index 729bf83685..7664ab58bd 100644 > > --- a/libavformat/rtpdec.c > > +++ b/libavformat/rtpdec.c > > @@ -633,7 +633,8 @@ 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; > > + int64_t rtcp_time, delta_time; > > + int32_t delta_timestamp; > > > > AVProducerReferenceTime *prft = > > (AVProducerReferenceTime *) av_packet_new_side_data( > > @@ -642,7 +643,7 @@ static int rtp_set_prft(RTPDemuxContext *s, AVPacket *pkt, uint32_t timestamp) { > > return AVERROR(ENOMEM); > > > > 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_timestamp = (int32_t)(timestamp - 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; > > will apply Thank you I found also another place where this rollover could be problematic diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c index 7664ab58bd..aa1ea46469 100644 --- a/libavformat/rtpdec.c +++ b/libavformat/rtpdec.c @@ -670,10 +670,10 @@ static void finalize_packet(RTPDemuxContext *s, AVPacket *pkt, uint32_t timestam if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE && s->ic->nb_streams > 1) { int64_t addend; - int delta_timestamp; + int32_t delta_timestamp; /* compute pts from timestamp with received ntp_time */ - delta_timestamp = timestamp - s->last_rtcp_timestamp; + delta_timestamp = (int32_t)(timestamp - s->last_rtcp_timestamp); /* convert to the PTS timebase */ addend = av_rescale(s->last_rtcp_ntp_time - s->first_rtcp_ntp_time, s->st->time_base.den, But to be honest I don't have time to properly test it, but I could send a patch with an RFT tag if you want. Regards, > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Asymptotically faster algorithms should always be preferred if you have > asymptotical amounts of data > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] rtpdec: Fix RTP timestamp wraparound in Producer Reference Time 2025-06-08 7:27 ` Clément Péron @ 2025-06-08 20:09 ` Michael Niedermayer 2025-06-09 11:54 ` Clément Péron 0 siblings, 1 reply; 6+ messages in thread From: Michael Niedermayer @ 2025-06-08 20:09 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 4589 bytes --] On Sun, Jun 08, 2025 at 09:27:07AM +0200, Clément Péron wrote: > Hi, > > On Sun, 8 Jun 2025 at 01:33, Michael Niedermayer <michael@niedermayer.cc> wrote: > > > > Hi > > > > On Sat, Jun 07, 2025 at 10:06:19AM +0200, Clément Péron wrote: > > > The rtp_set_prft() function incorrectly calculates the timestamp delta > > > when RTP timestamps wrap around the 32-bit boundary. The current code: > > > > > > delta_timestamp = (int64_t)timestamp - (int64_t)s->last_rtcp_timestamp; > > > > > > treats both timestamps as large positive values, causing wraparound to > > > produce a large negative delta instead of the correct small positive delta. > > > > > > For example, with a 90kHz video clock: > > > - last_rtcp_timestamp = 0xFFFFFF00 (near wraparound) > > > - timestamp = 0x00000100 (after wraparound) > > > - Current result: delta ≈ -4.3 billion ticks ≈ -47,721 seconds > > > - Expected result: delta ≈ +512 ticks ≈ +0.006 seconds > > > > > > This causes prft->wallclock to jump backward by approximately: > > > - 90kHz video: ~47,721 seconds (~13.25 hours) > > > - 48kHz audio: ~89,478 seconds (~24.9 hours) > > > - 8kHz audio: ~536,871 seconds (~6.2 days) > > > > > > Fix by casting the subtraction result to int32_t, which correctly > > > handles wraparound through modular arithmetic: > > > > > > delta_timestamp = (int32_t)(timestamp - s->last_rtcp_timestamp); > > > > > > This ensures the delta is always in the range [-2^31, 2^31-1], making > > > wraparound produce the correct small positive values. > > > > > > Fixes timing jumps in applications that rely on Producer Reference Time > > > for media synchronization. > > > > > > Signed-off-by: Clément Péron <peron.clem@gmail.com> > > > --- > > > libavformat/rtpdec.c | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c > > > index 729bf83685..7664ab58bd 100644 > > > --- a/libavformat/rtpdec.c > > > +++ b/libavformat/rtpdec.c > > > @@ -633,7 +633,8 @@ 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; > > > + int64_t rtcp_time, delta_time; > > > + int32_t delta_timestamp; > > > > > > AVProducerReferenceTime *prft = > > > (AVProducerReferenceTime *) av_packet_new_side_data( > > > @@ -642,7 +643,7 @@ static int rtp_set_prft(RTPDemuxContext *s, AVPacket *pkt, uint32_t timestamp) { > > > return AVERROR(ENOMEM); > > > > > > 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_timestamp = (int32_t)(timestamp - 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; > > > > will apply > > Thank you > > I found also another place where this rollover could be problematic > > diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c > index 7664ab58bd..aa1ea46469 100644 > --- a/libavformat/rtpdec.c > +++ b/libavformat/rtpdec.c > @@ -670,10 +670,10 @@ static void finalize_packet(RTPDemuxContext *s, > AVPacket *pkt, uint32_t timestam > > if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE && s->ic->nb_streams > 1) { > int64_t addend; > - int delta_timestamp; > + int32_t delta_timestamp; > > /* compute pts from timestamp with received ntp_time */ > - delta_timestamp = timestamp - s->last_rtcp_timestamp; > + delta_timestamp = (int32_t)(timestamp - s->last_rtcp_timestamp); > /* convert to the PTS timebase */ > addend = av_rescale(s->last_rtcp_ntp_time - s->first_rtcp_ntp_time, > s->st->time_base.den, > > But to be honest I don't have time to properly test it, but I could > send a patch with an RFT tag if you want. better to have a patch than no patch, also this only affects 64bit int systems. I would tend to apply this fix even if noone tests it. Or am i missing something ? Its just more correct after the patch thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The difference between a dictatorship and a democracy is that every 4 years the population together is allowed to provide 1 bit of input to the government. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] [-- Attachment #2: Type: text/plain, Size: 251 bytes --] _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] rtpdec: Fix RTP timestamp wraparound in Producer Reference Time 2025-06-08 20:09 ` Michael Niedermayer @ 2025-06-09 11:54 ` Clément Péron 0 siblings, 0 replies; 6+ messages in thread From: Clément Péron @ 2025-06-09 11:54 UTC (permalink / raw) To: FFmpeg development discussions and patches On Sun, 8 Jun 2025 at 22:09, Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Sun, Jun 08, 2025 at 09:27:07AM +0200, Clément Péron wrote: > > Hi, > > > > On Sun, 8 Jun 2025 at 01:33, Michael Niedermayer <michael@niedermayer.cc> wrote: > > > > > > Hi > > > > > > On Sat, Jun 07, 2025 at 10:06:19AM +0200, Clément Péron wrote: > > > > The rtp_set_prft() function incorrectly calculates the timestamp delta > > > > when RTP timestamps wrap around the 32-bit boundary. The current code: > > > > > > > > delta_timestamp = (int64_t)timestamp - (int64_t)s->last_rtcp_timestamp; > > > > > > > > treats both timestamps as large positive values, causing wraparound to > > > > produce a large negative delta instead of the correct small positive delta. > > > > > > > > For example, with a 90kHz video clock: > > > > - last_rtcp_timestamp = 0xFFFFFF00 (near wraparound) > > > > - timestamp = 0x00000100 (after wraparound) > > > > - Current result: delta ≈ -4.3 billion ticks ≈ -47,721 seconds > > > > - Expected result: delta ≈ +512 ticks ≈ +0.006 seconds > > > > > > > > This causes prft->wallclock to jump backward by approximately: > > > > - 90kHz video: ~47,721 seconds (~13.25 hours) > > > > - 48kHz audio: ~89,478 seconds (~24.9 hours) > > > > - 8kHz audio: ~536,871 seconds (~6.2 days) > > > > > > > > Fix by casting the subtraction result to int32_t, which correctly > > > > handles wraparound through modular arithmetic: > > > > > > > > delta_timestamp = (int32_t)(timestamp - s->last_rtcp_timestamp); > > > > > > > > This ensures the delta is always in the range [-2^31, 2^31-1], making > > > > wraparound produce the correct small positive values. > > > > > > > > Fixes timing jumps in applications that rely on Producer Reference Time > > > > for media synchronization. > > > > > > > > Signed-off-by: Clément Péron <peron.clem@gmail.com> > > > > --- > > > > libavformat/rtpdec.c | 5 +++-- > > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c > > > > index 729bf83685..7664ab58bd 100644 > > > > --- a/libavformat/rtpdec.c > > > > +++ b/libavformat/rtpdec.c > > > > @@ -633,7 +633,8 @@ 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; > > > > + int64_t rtcp_time, delta_time; > > > > + int32_t delta_timestamp; > > > > > > > > AVProducerReferenceTime *prft = > > > > (AVProducerReferenceTime *) av_packet_new_side_data( > > > > @@ -642,7 +643,7 @@ static int rtp_set_prft(RTPDemuxContext *s, AVPacket *pkt, uint32_t timestamp) { > > > > return AVERROR(ENOMEM); > > > > > > > > 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_timestamp = (int32_t)(timestamp - 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; > > > > > > will apply > > > > Thank you > > > > I found also another place where this rollover could be problematic > > > > diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c > > index 7664ab58bd..aa1ea46469 100644 > > --- a/libavformat/rtpdec.c > > +++ b/libavformat/rtpdec.c > > @@ -670,10 +670,10 @@ static void finalize_packet(RTPDemuxContext *s, > > AVPacket *pkt, uint32_t timestam > > > > if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE && s->ic->nb_streams > 1) { > > int64_t addend; > > - int delta_timestamp; > > + int32_t delta_timestamp; > > > > /* compute pts from timestamp with received ntp_time */ > > - delta_timestamp = timestamp - s->last_rtcp_timestamp; > > + delta_timestamp = (int32_t)(timestamp - s->last_rtcp_timestamp); > > /* convert to the PTS timebase */ > > addend = av_rescale(s->last_rtcp_ntp_time - s->first_rtcp_ntp_time, > > s->st->time_base.den, > > > > But to be honest I don't have time to properly test it, but I could > > send a patch with an RFT tag if you want. > > better to have a patch than no patch, also this only affects 64bit int > systems. I would tend to apply this fix even if noone tests it. > Or am i missing something ? Its just more correct after the patch You're correct I have sent another patch + add comment regarding Kieran feedback. https://patchwork.ffmpeg.org/project/ffmpeg/patch/20250608150737.73929-1-peron.clem@gmail.com/ > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > The difference between a dictatorship and a democracy is that every 4 years > the population together is allowed to provide 1 bit of input to the government. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] rtpdec: Fix RTP timestamp wraparound in Producer Reference Time 2025-06-07 8:06 [FFmpeg-devel] [PATCH v2] rtpdec: Fix RTP timestamp wraparound in Producer Reference Time Clément Péron 2025-06-07 23:33 ` Michael Niedermayer @ 2025-06-08 14:53 ` Kieran Kunhya via ffmpeg-devel 1 sibling, 0 replies; 6+ messages in thread From: Kieran Kunhya via ffmpeg-devel @ 2025-06-08 14:53 UTC (permalink / raw) To: FFmpeg development discussions and patches Cc: Kieran Kunhya, Clément Péron On Sat, Jun 7, 2025 at 9:06 AM Clément Péron <peron.clem-at-gmail.com@ffmpeg.org> wrote: > > The rtp_set_prft() function incorrectly calculates the timestamp delta > when RTP timestamps wrap around the 32-bit boundary. The current code: > > delta_timestamp = (int64_t)timestamp - (int64_t)s->last_rtcp_timestamp; > > treats both timestamps as large positive values, causing wraparound to > produce a large negative delta instead of the correct small positive delta. > > For example, with a 90kHz video clock: > - last_rtcp_timestamp = 0xFFFFFF00 (near wraparound) > - timestamp = 0x00000100 (after wraparound) > - Current result: delta ≈ -4.3 billion ticks ≈ -47,721 seconds > - Expected result: delta ≈ +512 ticks ≈ +0.006 seconds > > This causes prft->wallclock to jump backward by approximately: > - 90kHz video: ~47,721 seconds (~13.25 hours) > - 48kHz audio: ~89,478 seconds (~24.9 hours) > - 8kHz audio: ~536,871 seconds (~6.2 days) > > Fix by casting the subtraction result to int32_t, which correctly > handles wraparound through modular arithmetic: > > delta_timestamp = (int32_t)(timestamp - s->last_rtcp_timestamp); > > This ensures the delta is always in the range [-2^31, 2^31-1], making > wraparound produce the correct small positive values. > > Fixes timing jumps in applications that rely on Producer Reference Time > for media synchronization. > > Signed-off-by: Clément Péron <peron.clem@gmail.com> > --- > libavformat/rtpdec.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c > index 729bf83685..7664ab58bd 100644 > --- a/libavformat/rtpdec.c > +++ b/libavformat/rtpdec.c > @@ -633,7 +633,8 @@ 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; > + int64_t rtcp_time, delta_time; > + int32_t delta_timestamp; > > AVProducerReferenceTime *prft = > (AVProducerReferenceTime *) av_packet_new_side_data( > @@ -642,7 +643,7 @@ static int rtp_set_prft(RTPDemuxContext *s, AVPacket *pkt, uint32_t timestamp) { > return AVERROR(ENOMEM); > > 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_timestamp = (int32_t)(timestamp - 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; > -- > 2.48.1 Seems fine, a comment might help explain the wrapround logic. Kieran _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-06-09 11:54 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-06-07 8:06 [FFmpeg-devel] [PATCH v2] rtpdec: Fix RTP timestamp wraparound in Producer Reference Time Clément Péron 2025-06-07 23:33 ` Michael Niedermayer 2025-06-08 7:27 ` Clément Péron 2025-06-08 20:09 ` Michael Niedermayer 2025-06-09 11:54 ` Clément Péron 2025-06-08 14:53 ` Kieran Kunhya via ffmpeg-devel
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git