diff mbox series

[FFmpeg-devel] avformat: set start_time_realtime when demuxing

Message ID 20220614080937.5263-1-ffmpeg@gyani.pro
State New
Headers show
Series [FFmpeg-devel] avformat: set start_time_realtime when demuxing | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_armv7_RPi4 success Make finished
andriy/make_fate_armv7_RPi4 success Make fate finished

Commit Message

Gyan Doshi June 14, 2022, 8:09 a.m. UTC 
Add new flag AVFMT_START_REALTIME for the couple of demuxers that set it
internally.
---
 doc/APIchanges         | 3 +++
 libavformat/avformat.h | 1 +
 libavformat/demux.c    | 3 +++
 libavformat/rtsp.c     | 3 ++-
 libavformat/version.h  | 2 +-
 5 files changed, 10 insertions(+), 2 deletions(-)

Comments

Marton Balint June 14, 2022, 8:21 a.m. UTC | #1 
On Tue, 14 Jun 2022, Gyan Doshi wrote:

> Add new flag AVFMT_START_REALTIME for the couple of demuxers that set it
> internally.

I don't think this is the same semantics.

start_time_realtime is the time when the stream was originally captured, 
and not when the stream was received. So you can't simply use av_gettime() 
in the demuxers for that field.

Regards.
Marton



> ---
> doc/APIchanges         | 3 +++
> libavformat/avformat.h | 1 +
> libavformat/demux.c    | 3 +++
> libavformat/rtsp.c     | 3 ++-
> libavformat/version.h  | 2 +-
> 5 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 5857e67ae6..4aa9e36777 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,9 @@ libavutil:     2021-04-27
>
> API changes, most recent first:
>
> +2022-06-xx - xxxxxxxxxx - lavf 59.26.100 - avformat.h
> +  Add AVFMT_START_REALTIME flag.
> +
> 2022-06-12 - xxxxxxxxxx - lavf 59.25.100 - avio.h
>   Add avio_vprintf(), similar to avio_printf() but allow to use it
>   from within a function taking a variable argument list as input.
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index f12fa7d904..4e247d68fc 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -499,6 +499,7 @@ typedef struct AVProbeData {
>                                         The user or muxer can override this through
>                                         AVFormatContext.avoid_negative_ts
>                                         */
> +#define AVFMT_START_REALTIME 0x80000 /**< Demuxer sets start_time_realtime */
>
> #define AVFMT_SEEK_TO_PTS   0x4000000 /**< Seeking is based on PTS */
>
> diff --git a/libavformat/demux.c b/libavformat/demux.c
> index 1620716716..28c6966e71 100644
> --- a/libavformat/demux.c
> +++ b/libavformat/demux.c
> @@ -628,6 +628,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>
>         force_codec_ids(s, st);
>
> +        if (!(s->iformat->flags & AVFMT_START_REALTIME) && s->start_time_realtime == AV_NOPTS_VALUE)
> +            s->start_time_realtime = av_gettime();
> +
>         /* TODO: audio: time filter; video: frame reordering (pts != dts) */
>         if (s->use_wallclock_as_timestamps)
>             pkt->dts = pkt->pts = av_rescale_q(av_gettime(), AV_TIME_BASE_Q, st->time_base);
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index 88e9ef5226..2cafe33887 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -2489,6 +2489,7 @@ const AVInputFormat ff_sdp_demuxer = {
>     .read_header    = sdp_read_header,
>     .read_packet    = ff_rtsp_fetch_packet,
>     .read_close     = sdp_read_close,
> +    .flags          = AVFMT_START_REALTIME,
>     .priv_class     = &sdp_demuxer_class,
> };
> #endif /* CONFIG_SDP_DEMUXER */
> @@ -2648,7 +2649,7 @@ const AVInputFormat ff_rtp_demuxer = {
>     .read_header    = rtp_read_header,
>     .read_packet    = ff_rtsp_fetch_packet,
>     .read_close     = sdp_read_close,
> -    .flags          = AVFMT_NOFILE,
> +    .flags          = AVFMT_NOFILE | AVFMT_START_REALTIME,
>     .priv_class     = &rtp_demuxer_class,
> };
> #endif /* CONFIG_RTP_DEMUXER */
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 966ebb7ed3..0708d619c0 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -31,7 +31,7 @@
>
> #include "version_major.h"
>
> -#define LIBAVFORMAT_VERSION_MINOR  25
> +#define LIBAVFORMAT_VERSION_MINOR  26
> #define LIBAVFORMAT_VERSION_MICRO 100
>
> #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> -- 
> 2.36.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".
>
Gyan Doshi June 14, 2022, 9:11 a.m. UTC | #2 
On 2022-06-14 01:51 pm, Marton Balint wrote:
>
>
> On Tue, 14 Jun 2022, Gyan Doshi wrote:
>
>> Add new flag AVFMT_START_REALTIME for the couple of demuxers that set it
>> internally.
>
> I don't think this is the same semantics.
>
> start_time_realtime is the time when the stream was originally 
> captured, and not when the stream was received. So you can't simply 
> use av_gettime() in the demuxers for that field.

I saw that in the rtsp read_packet. I thought of adding a new field but 
think the semantics are close enough to serve the purpose of a rough 
sync so I added the flag for the demuxers that receive origin timestamp.

I can update the doxy to record this difference. Or should I add a new 
field?

Regards,
Gyan
Marton Balint June 14, 2022, 8:26 p.m. UTC | #3 
On Tue, 14 Jun 2022, Gyan Doshi wrote:

>
>
> On 2022-06-14 01:51 pm, Marton Balint wrote:
>> 
>>
>>  On Tue, 14 Jun 2022, Gyan Doshi wrote:
>>
>>>  Add new flag AVFMT_START_REALTIME for the couple of demuxers that set it
>>>  internally.
>>
>>  I don't think this is the same semantics.
>>
>>  start_time_realtime is the time when the stream was originally captured,
>>  and not when the stream was received. So you can't simply use av_gettime()
>>  in the demuxers for that field.
>
> I saw that in the rtsp read_packet. I thought of adding a new field but think 
> the semantics are close enough to serve the purpose of a rough sync so I 
> added the flag for the demuxers that receive origin timestamp.
>
> I can update the doxy to record this difference. Or should I add a new field?

I prefer a new field.

Thanks,
Marton
Andreas Rheinhardt June 14, 2022, 9:48 p.m. UTC | #4 
Gyan Doshi:
> Add new flag AVFMT_START_REALTIME for the couple of demuxers that set it
> internally.
> ---
>  doc/APIchanges         | 3 +++
>  libavformat/avformat.h | 1 +
>  libavformat/demux.c    | 3 +++
>  libavformat/rtsp.c     | 3 ++-
>  libavformat/version.h  | 2 +-
>  5 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 5857e67ae6..4aa9e36777 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,9 @@ libavutil:     2021-04-27
>  
>  API changes, most recent first:
>  
> +2022-06-xx - xxxxxxxxxx - lavf 59.26.100 - avformat.h
> +  Add AVFMT_START_REALTIME flag.
> +
>  2022-06-12 - xxxxxxxxxx - lavf 59.25.100 - avio.h
>    Add avio_vprintf(), similar to avio_printf() but allow to use it
>    from within a function taking a variable argument list as input.
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index f12fa7d904..4e247d68fc 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -499,6 +499,7 @@ typedef struct AVProbeData {
>                                          The user or muxer can override this through
>                                          AVFormatContext.avoid_negative_ts
>                                          */
> +#define AVFMT_START_REALTIME 0x80000 /**< Demuxer sets start_time_realtime */

This is incorrect: If this were merged, start_time_realtime would always
be set by libavformat; whether it is done by the generic code or by
demuxer specific code is an implementation detail and irrelevant for the
user. Given that it is also set for demuxers without this flag actually
means that libavformat lies when using a demuxer without this flag.

>  
>  #define AVFMT_SEEK_TO_PTS   0x4000000 /**< Seeking is based on PTS */
>  
> diff --git a/libavformat/demux.c b/libavformat/demux.c
> index 1620716716..28c6966e71 100644
> --- a/libavformat/demux.c
> +++ b/libavformat/demux.c
> @@ -628,6 +628,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>  
>          force_codec_ids(s, st);
>  
> +        if (!(s->iformat->flags & AVFMT_START_REALTIME) && s->start_time_realtime == AV_NOPTS_VALUE)
> +            s->start_time_realtime = av_gettime();
> +
>          /* TODO: audio: time filter; video: frame reordering (pts != dts) */
>          if (s->use_wallclock_as_timestamps)
>              pkt->dts = pkt->pts = av_rescale_q(av_gettime(), AV_TIME_BASE_Q, st->time_base);
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index 88e9ef5226..2cafe33887 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -2489,6 +2489,7 @@ const AVInputFormat ff_sdp_demuxer = {
>      .read_header    = sdp_read_header,
>      .read_packet    = ff_rtsp_fetch_packet,
>      .read_close     = sdp_read_close,
> +    .flags          = AVFMT_START_REALTIME,
>      .priv_class     = &sdp_demuxer_class,
>  };
>  #endif /* CONFIG_SDP_DEMUXER */
> @@ -2648,7 +2649,7 @@ const AVInputFormat ff_rtp_demuxer = {
>      .read_header    = rtp_read_header,
>      .read_packet    = ff_rtsp_fetch_packet,
>      .read_close     = sdp_read_close,
> -    .flags          = AVFMT_NOFILE,
> +    .flags          = AVFMT_NOFILE | AVFMT_START_REALTIME,
>      .priv_class     = &rtp_demuxer_class,
>  };
>  #endif /* CONFIG_RTP_DEMUXER */
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 966ebb7ed3..0708d619c0 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -31,7 +31,7 @@
>  
>  #include "version_major.h"
>  
> -#define LIBAVFORMAT_VERSION_MINOR  25
> +#define LIBAVFORMAT_VERSION_MINOR  26
>  #define LIBAVFORMAT_VERSION_MICRO 100
>  
>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
Gyan Doshi June 15, 2022, 4:08 a.m. UTC | #5 
On 2022-06-15 03:18 am, Andreas Rheinhardt wrote:
> Gyan Doshi:
>> Add new flag AVFMT_START_REALTIME for the couple of demuxers that set it
>> internally.
>> ---
>>   doc/APIchanges         | 3 +++
>>   libavformat/avformat.h | 1 +
>>   libavformat/demux.c    | 3 +++
>>   libavformat/rtsp.c     | 3 ++-
>>   libavformat/version.h  | 2 +-
>>   5 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index 5857e67ae6..4aa9e36777 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -14,6 +14,9 @@ libavutil:     2021-04-27
>>   
>>   API changes, most recent first:
>>   
>> +2022-06-xx - xxxxxxxxxx - lavf 59.26.100 - avformat.h
>> +  Add AVFMT_START_REALTIME flag.
>> +
>>   2022-06-12 - xxxxxxxxxx - lavf 59.25.100 - avio.h
>>     Add avio_vprintf(), similar to avio_printf() but allow to use it
>>     from within a function taking a variable argument list as input.
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index f12fa7d904..4e247d68fc 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -499,6 +499,7 @@ typedef struct AVProbeData {
>>                                           The user or muxer can override this through
>>                                           AVFormatContext.avoid_negative_ts
>>                                           */
>> +#define AVFMT_START_REALTIME 0x80000 /**< Demuxer sets start_time_realtime */
> This is incorrect: If this were merged, start_time_realtime would always
> be set by libavformat; whether it is done by the generic code or by
> demuxer specific code is an implementation detail and irrelevant for the
> user. Given that it is also set for demuxers without this flag actually
> means that libavformat lies when using a demuxer without this flag.

This is moot since I'm switching to a new field but would either of 
these satisfy you?

s/Demuxer/Format/

or

/**< start_time_realtime is populated with conveyed origin wallclock timestamp */

Regards,
Gyan
Andreas Rheinhardt June 15, 2022, 4:17 a.m. UTC | #6 
Gyan Doshi:
> 
> 
> On 2022-06-15 03:18 am, Andreas Rheinhardt wrote:
>> Gyan Doshi:
>>> Add new flag AVFMT_START_REALTIME for the couple of demuxers that set it
>>> internally.
>>> ---
>>>   doc/APIchanges         | 3 +++
>>>   libavformat/avformat.h | 1 +
>>>   libavformat/demux.c    | 3 +++
>>>   libavformat/rtsp.c     | 3 ++-
>>>   libavformat/version.h  | 2 +-
>>>   5 files changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/doc/APIchanges b/doc/APIchanges
>>> index 5857e67ae6..4aa9e36777 100644
>>> --- a/doc/APIchanges
>>> +++ b/doc/APIchanges
>>> @@ -14,6 +14,9 @@ libavutil:     2021-04-27
>>>     API changes, most recent first:
>>>   +2022-06-xx - xxxxxxxxxx - lavf 59.26.100 - avformat.h
>>> +  Add AVFMT_START_REALTIME flag.
>>> +
>>>   2022-06-12 - xxxxxxxxxx - lavf 59.25.100 - avio.h
>>>     Add avio_vprintf(), similar to avio_printf() but allow to use it
>>>     from within a function taking a variable argument list as input.
>>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>> index f12fa7d904..4e247d68fc 100644
>>> --- a/libavformat/avformat.h
>>> +++ b/libavformat/avformat.h
>>> @@ -499,6 +499,7 @@ typedef struct AVProbeData {
>>>                                           The user or muxer can
>>> override this through
>>>                                          
>>> AVFormatContext.avoid_negative_ts
>>>                                           */
>>> +#define AVFMT_START_REALTIME 0x80000 /**< Demuxer sets
>>> start_time_realtime */
>> This is incorrect: If this were merged, start_time_realtime would always
>> be set by libavformat; whether it is done by the generic code or by
>> demuxer specific code is an implementation detail and irrelevant for the
>> user. Given that it is also set for demuxers without this flag actually
>> means that libavformat lies when using a demuxer without this flag.
> 
> This is moot since I'm switching to a new field but would either of
> these satisfy you?
> 
> s/Demuxer/Format/
> 
> or
> 
> /**< start_time_realtime is populated with conveyed origin wallclock
> timestamp */
> 

Of course not. My point was not about the semantics of the field being
populated, but about the flag and the fact that the field was populated
for demuxers that don't have this flag set. To remedy this, you should
stop setting start_time_realtime for demuxers that don't have this flag set.

- Andreas
diff mbox series

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index 5857e67ae6..4aa9e36777 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,9 @@  libavutil:     2021-04-27
 
 API changes, most recent first:
 
+2022-06-xx - xxxxxxxxxx - lavf 59.26.100 - avformat.h
+  Add AVFMT_START_REALTIME flag.
+
 2022-06-12 - xxxxxxxxxx - lavf 59.25.100 - avio.h
   Add avio_vprintf(), similar to avio_printf() but allow to use it
   from within a function taking a variable argument list as input.
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index f12fa7d904..4e247d68fc 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -499,6 +499,7 @@  typedef struct AVProbeData {
                                         The user or muxer can override this through
                                         AVFormatContext.avoid_negative_ts
                                         */
+#define AVFMT_START_REALTIME 0x80000 /**< Demuxer sets start_time_realtime */
 
 #define AVFMT_SEEK_TO_PTS   0x4000000 /**< Seeking is based on PTS */
 
diff --git a/libavformat/demux.c b/libavformat/demux.c
index 1620716716..28c6966e71 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -628,6 +628,9 @@  FF_ENABLE_DEPRECATION_WARNINGS
 
         force_codec_ids(s, st);
 
+        if (!(s->iformat->flags & AVFMT_START_REALTIME) && s->start_time_realtime == AV_NOPTS_VALUE)
+            s->start_time_realtime = av_gettime();
+
         /* TODO: audio: time filter; video: frame reordering (pts != dts) */
         if (s->use_wallclock_as_timestamps)
             pkt->dts = pkt->pts = av_rescale_q(av_gettime(), AV_TIME_BASE_Q, st->time_base);
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 88e9ef5226..2cafe33887 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -2489,6 +2489,7 @@  const AVInputFormat ff_sdp_demuxer = {
     .read_header    = sdp_read_header,
     .read_packet    = ff_rtsp_fetch_packet,
     .read_close     = sdp_read_close,
+    .flags          = AVFMT_START_REALTIME,
     .priv_class     = &sdp_demuxer_class,
 };
 #endif /* CONFIG_SDP_DEMUXER */
@@ -2648,7 +2649,7 @@  const AVInputFormat ff_rtp_demuxer = {
     .read_header    = rtp_read_header,
     .read_packet    = ff_rtsp_fetch_packet,
     .read_close     = sdp_read_close,
-    .flags          = AVFMT_NOFILE,
+    .flags          = AVFMT_NOFILE | AVFMT_START_REALTIME,
     .priv_class     = &rtp_demuxer_class,
 };
 #endif /* CONFIG_RTP_DEMUXER */
diff --git a/libavformat/version.h b/libavformat/version.h
index 966ebb7ed3..0708d619c0 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -31,7 +31,7 @@ 
 
 #include "version_major.h"
 
-#define LIBAVFORMAT_VERSION_MINOR  25
+#define LIBAVFORMAT_VERSION_MINOR  26
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \