diff mbox series

[FFmpeg-devel] avformat/rtpdec: attach producer reference time if available

Message ID 20210318002830.469769-1-alokpr@gmail.com
State New
Headers show
Series [FFmpeg-devel] avformat/rtpdec: attach producer reference time if available | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Alok Priyadarshi March 18, 2021, 12:28 a.m. UTC
This produces true wallclock time at rtp source instead of the
local wallclock time at rtp client.
---
 libavformat/internal.h |  8 ++++++++
 libavformat/rtpdec.c   | 10 ++++++++++
 libavformat/utils.c    |  9 +++++++++
 3 files changed, 27 insertions(+)

Comments

Alok Priyadarshi March 22, 2021, 4:24 p.m. UTC | #1
This is my first ffmpeg patch, so I am not sure what else I need to do to
get it reviewed and submitted. Could someone please let me know? Thanks!

On Wed, Mar 17, 2021 at 5:28 PM Alok Priyadarshi <alokpr@gmail.com> wrote:

> This produces true wallclock time at rtp source instead of the
> local wallclock time at rtp client.
> ---
>  libavformat/internal.h |  8 ++++++++
>  libavformat/rtpdec.c   | 10 ++++++++++
>  libavformat/utils.c    |  9 +++++++++
>  3 files changed, 27 insertions(+)
>
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index 17a6ab07d3..1e10cde00e 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -254,6 +254,14 @@ uint64_t ff_ntp_time(void);
>   */
>  uint64_t ff_get_formatted_ntp_time(uint64_t ntp_time_us);
>
> +/**
> + * Parse the NTP time in micro seconds (since NTP epoch).
> + *
> + * @param ntp_ts NTP time stamp formatted as per the RFC-5905.
> + * @return the time in micro seconds (since NTP epoch)
> + */
> +uint64_t ff_parse_ntp_time(uint64_t ntp_ts);
> +
>  /**
>   * Append the media-specific SDP fragment for the media stream c
>   * to the buffer buff.
> diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
> index 3d5b200099..3e5f280f2f 100644
> --- a/libavformat/rtpdec.c
> +++ b/libavformat/rtpdec.c
> @@ -23,6 +23,7 @@
>  #include "libavutil/avstring.h"
>  #include "libavutil/intreadwrite.h"
>  #include "libavutil/time.h"
> +#include "libavcodec/packet_internal.h"
>
>  #include "avformat.h"
>  #include "network.h"
> @@ -30,6 +31,7 @@
>  #include "url.h"
>  #include "rtpdec.h"
>  #include "rtpdec_formats.h"
> +#include "internal.h"
>
>  #define MIN_FEEDBACK_INTERVAL 200000 /* 200 ms in us */
>
> @@ -594,6 +596,14 @@ static void finalize_packet(RTPDemuxContext *s,
> AVPacket *pkt, uint32_t timestam
>      if (timestamp == RTP_NOTS_VALUE)
>          return;
>
> +    if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE) {
> +        uint64_t prft_us = ff_parse_ntp_time(s->last_rtcp_ntp_time) -
> NTP_OFFSET_US +
> +                           timestamp - s->last_rtcp_timestamp;
> +        if (ff_side_data_set_prft(pkt, prft_us) < 0) {
> +            av_log(s->ic, AV_LOG_WARNING, "rtpdec: failed to set prft");
> +        }
> +    }
> +
>      if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE && s->ic->nb_streams > 1)
> {
>          int64_t addend;
>          int delta_timestamp;
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 7e5767ec60..569922beaf 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -4734,6 +4734,15 @@ uint64_t ff_get_formatted_ntp_time(uint64_t
> ntp_time_us)
>      return ntp_ts;
>  }
>
> +uint64_t ff_parse_ntp_time(uint64_t ntp_ts)
> +{
> +    uint64_t sec = ntp_ts >> 32;
> +    uint64_t frac_part = ntp_ts & 0xFFFFFFFFULL;
> +    uint64_t usec = (frac_part * 1000000) / 0xFFFFFFFFULL;
> +
> +    return (sec * 1000000) + usec;
> +}
> +
>  int av_get_frame_filename2(char *buf, int buf_size, const char *path, int
> number, int flags)
>  {
>      const char *p;
> --
> 2.25.1
>
>
James Almer March 22, 2021, 6:46 p.m. UTC | #2
On 3/17/2021 9:28 PM, Alok Priyadarshi wrote:
> This produces true wallclock time at rtp source instead of the
> local wallclock time at rtp client.
> ---
>   libavformat/internal.h |  8 ++++++++
>   libavformat/rtpdec.c   | 10 ++++++++++
>   libavformat/utils.c    |  9 +++++++++
>   3 files changed, 27 insertions(+)
> 
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index 17a6ab07d3..1e10cde00e 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -254,6 +254,14 @@ uint64_t ff_ntp_time(void);
>    */
>   uint64_t ff_get_formatted_ntp_time(uint64_t ntp_time_us);
>   
> +/**
> + * Parse the NTP time in micro seconds (since NTP epoch).
> + *
> + * @param ntp_ts NTP time stamp formatted as per the RFC-5905.
> + * @return the time in micro seconds (since NTP epoch)
> + */
> +uint64_t ff_parse_ntp_time(uint64_t ntp_ts);
> +
>   /**
>    * Append the media-specific SDP fragment for the media stream c
>    * to the buffer buff.
> diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
> index 3d5b200099..3e5f280f2f 100644
> --- a/libavformat/rtpdec.c
> +++ b/libavformat/rtpdec.c
> @@ -23,6 +23,7 @@
>   #include "libavutil/avstring.h"
>   #include "libavutil/intreadwrite.h"
>   #include "libavutil/time.h"
> +#include "libavcodec/packet_internal.h"
>   
>   #include "avformat.h"
>   #include "network.h"
> @@ -30,6 +31,7 @@
>   #include "url.h"
>   #include "rtpdec.h"
>   #include "rtpdec_formats.h"
> +#include "internal.h"
>   
>   #define MIN_FEEDBACK_INTERVAL 200000 /* 200 ms in us */
>   
> @@ -594,6 +596,14 @@ static void finalize_packet(RTPDemuxContext *s, AVPacket *pkt, uint32_t timestam
>       if (timestamp == RTP_NOTS_VALUE)
>           return;
>   
> +    if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE) {
> +        uint64_t prft_us = ff_parse_ntp_time(s->last_rtcp_ntp_time) - NTP_OFFSET_US +
> +                           timestamp - s->last_rtcp_timestamp;
> +        if (ff_side_data_set_prft(pkt, prft_us) < 0) {

This is a libavcodec function. It can't be used from libavformat. You 
should instead manually add the side data using 
av_packet_new_side_data(pkt, AV_PKT_DATA_PRFT, 
sizeof(AVProducerReferenceTime)), then filling both the struct's 
wallclock and flags fields following the definition in the struct's doxy 
and ISO/IEC 14496-12, so flags should probably be 24.

That being said, what is expected to use this value? Decoders don't care 
about prft, so is your usecase a remuxing one?

> +            av_log(s->ic, AV_LOG_WARNING, "rtpdec: failed to set prft");
> +        }
> +    }
> +
>       if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE && s->ic->nb_streams > 1) {
>           int64_t addend;
>           int delta_timestamp;
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 7e5767ec60..569922beaf 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -4734,6 +4734,15 @@ uint64_t ff_get_formatted_ntp_time(uint64_t ntp_time_us)
>       return ntp_ts;
>   }
>   
> +uint64_t ff_parse_ntp_time(uint64_t ntp_ts)
> +{
> +    uint64_t sec = ntp_ts >> 32;
> +    uint64_t frac_part = ntp_ts & 0xFFFFFFFFULL;
> +    uint64_t usec = (frac_part * 1000000) / 0xFFFFFFFFULL;
> +
> +    return (sec * 1000000) + usec;
> +}
> +
>   int av_get_frame_filename2(char *buf, int buf_size, const char *path, int number, int flags)
>   {
>       const char *p;
>
Alok Priyadarshi March 23, 2021, 8 p.m. UTC | #3
James,

Thanks for reviewing the patch. I have addressed your comments in the new
patch.

You are right - the use case is remuxing in particular rtsp stream to
fragmented mp4 using the following command:
ffmpeg -i <rtsp-url> -c copy -movflags
frag_keyframe+empty_moov+default_base_moof -write_prft wallclock -f mp4
pipe:1

Without this patch, the prft box contains the local wallclock time instead
of that at the rtsp source.

On Mon, Mar 22, 2021 at 11:46 AM James Almer <jamrial@gmail.com> wrote:

> On 3/17/2021 9:28 PM, Alok Priyadarshi wrote:
> > This produces true wallclock time at rtp source instead of the
> > local wallclock time at rtp client.
> > ---
> >   libavformat/internal.h |  8 ++++++++
> >   libavformat/rtpdec.c   | 10 ++++++++++
> >   libavformat/utils.c    |  9 +++++++++
> >   3 files changed, 27 insertions(+)
> >
> > diff --git a/libavformat/internal.h b/libavformat/internal.h
> > index 17a6ab07d3..1e10cde00e 100644
> > --- a/libavformat/internal.h
> > +++ b/libavformat/internal.h
> > @@ -254,6 +254,14 @@ uint64_t ff_ntp_time(void);
> >    */
> >   uint64_t ff_get_formatted_ntp_time(uint64_t ntp_time_us);
> >
> > +/**
> > + * Parse the NTP time in micro seconds (since NTP epoch).
> > + *
> > + * @param ntp_ts NTP time stamp formatted as per the RFC-5905.
> > + * @return the time in micro seconds (since NTP epoch)
> > + */
> > +uint64_t ff_parse_ntp_time(uint64_t ntp_ts);
> > +
> >   /**
> >    * Append the media-specific SDP fragment for the media stream c
> >    * to the buffer buff.
> > diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
> > index 3d5b200099..3e5f280f2f 100644
> > --- a/libavformat/rtpdec.c
> > +++ b/libavformat/rtpdec.c
> > @@ -23,6 +23,7 @@
> >   #include "libavutil/avstring.h"
> >   #include "libavutil/intreadwrite.h"
> >   #include "libavutil/time.h"
> > +#include "libavcodec/packet_internal.h"
> >
> >   #include "avformat.h"
> >   #include "network.h"
> > @@ -30,6 +31,7 @@
> >   #include "url.h"
> >   #include "rtpdec.h"
> >   #include "rtpdec_formats.h"
> > +#include "internal.h"
> >
> >   #define MIN_FEEDBACK_INTERVAL 200000 /* 200 ms in us */
> >
> > @@ -594,6 +596,14 @@ static void finalize_packet(RTPDemuxContext *s,
> AVPacket *pkt, uint32_t timestam
> >       if (timestamp == RTP_NOTS_VALUE)
> >           return;
> >
> > +    if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE) {
> > +        uint64_t prft_us = ff_parse_ntp_time(s->last_rtcp_ntp_time) -
> NTP_OFFSET_US +
> > +                           timestamp - s->last_rtcp_timestamp;
> > +        if (ff_side_data_set_prft(pkt, prft_us) < 0) {
>
> This is a libavcodec function. It can't be used from libavformat. You
> should instead manually add the side data using
> av_packet_new_side_data(pkt, AV_PKT_DATA_PRFT,
> sizeof(AVProducerReferenceTime)), then filling both the struct's
> wallclock and flags fields following the definition in the struct's doxy
> and ISO/IEC 14496-12, so flags should probably be 24.
>
> That being said, what is expected to use this value? Decoders don't care
> about prft, so is your usecase a remuxing one?
>
> > +            av_log(s->ic, AV_LOG_WARNING, "rtpdec: failed to set prft");
> > +        }
> > +    }
> > +
> >       if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE && s->ic->nb_streams >
> 1) {
> >           int64_t addend;
> >           int delta_timestamp;
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index 7e5767ec60..569922beaf 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -4734,6 +4734,15 @@ uint64_t ff_get_formatted_ntp_time(uint64_t
> ntp_time_us)
> >       return ntp_ts;
> >   }
> >
> > +uint64_t ff_parse_ntp_time(uint64_t ntp_ts)
> > +{
> > +    uint64_t sec = ntp_ts >> 32;
> > +    uint64_t frac_part = ntp_ts & 0xFFFFFFFFULL;
> > +    uint64_t usec = (frac_part * 1000000) / 0xFFFFFFFFULL;
> > +
> > +    return (sec * 1000000) + usec;
> > +}
> > +
> >   int av_get_frame_filename2(char *buf, int buf_size, const char *path,
> int number, int flags)
> >   {
> >       const char *p;
> >
>
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavformat/internal.h b/libavformat/internal.h
index 17a6ab07d3..1e10cde00e 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -254,6 +254,14 @@  uint64_t ff_ntp_time(void);
  */
 uint64_t ff_get_formatted_ntp_time(uint64_t ntp_time_us);
 
+/**
+ * Parse the NTP time in micro seconds (since NTP epoch).
+ *
+ * @param ntp_ts NTP time stamp formatted as per the RFC-5905.
+ * @return the time in micro seconds (since NTP epoch)
+ */
+uint64_t ff_parse_ntp_time(uint64_t ntp_ts);
+
 /**
  * Append the media-specific SDP fragment for the media stream c
  * to the buffer buff.
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index 3d5b200099..3e5f280f2f 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -23,6 +23,7 @@ 
 #include "libavutil/avstring.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/time.h"
+#include "libavcodec/packet_internal.h"
 
 #include "avformat.h"
 #include "network.h"
@@ -30,6 +31,7 @@ 
 #include "url.h"
 #include "rtpdec.h"
 #include "rtpdec_formats.h"
+#include "internal.h"
 
 #define MIN_FEEDBACK_INTERVAL 200000 /* 200 ms in us */
 
@@ -594,6 +596,14 @@  static void finalize_packet(RTPDemuxContext *s, AVPacket *pkt, uint32_t timestam
     if (timestamp == RTP_NOTS_VALUE)
         return;
 
+    if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE) {
+        uint64_t prft_us = ff_parse_ntp_time(s->last_rtcp_ntp_time) - NTP_OFFSET_US +
+                           timestamp - s->last_rtcp_timestamp;
+        if (ff_side_data_set_prft(pkt, prft_us) < 0) {
+            av_log(s->ic, AV_LOG_WARNING, "rtpdec: failed to set prft");
+        }
+    }
+
     if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE && s->ic->nb_streams > 1) {
         int64_t addend;
         int delta_timestamp;
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 7e5767ec60..569922beaf 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -4734,6 +4734,15 @@  uint64_t ff_get_formatted_ntp_time(uint64_t ntp_time_us)
     return ntp_ts;
 }
 
+uint64_t ff_parse_ntp_time(uint64_t ntp_ts)
+{
+    uint64_t sec = ntp_ts >> 32;
+    uint64_t frac_part = ntp_ts & 0xFFFFFFFFULL;
+    uint64_t usec = (frac_part * 1000000) / 0xFFFFFFFFULL;
+
+    return (sec * 1000000) + usec;
+}
+
 int av_get_frame_filename2(char *buf, int buf_size, const char *path, int number, int flags)
 {
     const char *p;