* [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-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
* 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
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