[FFmpeg-devel] [PATCH] avformat/dump: Change precision of stream start offsets

Tue Apr 22 22:25:49 EEST 2025

On Tue, 22 Apr 2025, Gyan Doshi wrote:

>
>
> On 2025-04-22 02:22 am, softworkz . wrote:
>>
>>>  -----Original Message-----
>>>  From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>>>  Marton Balint
>>>  Sent: Montag, 21. April 2025 22:18
>>>  To: FFmpeg development discussions and patches <ffmpeg-
>>>  devel at ffmpeg.org>
>>>  Subject: Re: [FFmpeg-devel] [PATCH] avformat/dump: Change precision of
>>>  stream start offsets
>>> 
>>> 
>>>
>>>  On Mon, 21 Apr 2025, softworkz . wrote:
>>> 
>>>>
>>>>>  -----Original Message-----
>>>>>  From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>>>>>  Marton Balint
>>>>>  Sent: Montag, 21. April 2025 21:32
>>>>>  To: FFmpeg development discussions and patches <ffmpeg-
>>>>>  devel at ffmpeg.org>
>>>>>  Subject: Re: [FFmpeg-devel] [PATCH] avformat/dump: Change precision
>>>  of
>>>>>  stream start offsets
>>>>> 
>>>>> 
>>>>>
>>>>>  On Mon, 21 Apr 2025, softworkz . wrote:
>>>>> 
>>>>>>
>>>>>>>  -----Original Message-----
>>>>>>>  From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>>>>>  Gyan
>>>>>>>  Doshi
>>>>>>>  Sent: Montag, 21. April 2025 06:51
>>>>>>>  To: ffmpeg-devel at ffmpeg.org
>>>>>>>  Subject: Re: [FFmpeg-devel] [PATCH] avformat/dump: Change
>>>  precision
>>>>>  of
>>>>>>>  stream start offsets
>>>>>>> 
>>>>>>> 
>>>>>>>
>>>>>>>  On 2025-04-21 01:41 am, softworkz wrote:
>>>>>>>>  From: softworkz <softworkz at hotmail.com>
>>>>>>>>
>>>>>>>>  Changing this to 6 digits to align with other
>>>>>>>>  printed times
>>>>>>>>
>>>>>>>>  Signed-off-by: softworkz <softworkz at hotmail.com>
>>>>>>>>  ---
>>>>>>>>        avformat/dump: Change precision of stream start offsets
>>>>>>>>
>>>>>>>>        Changing this to 6 digits to align with other printed times
>>>>>>>>
>>>>>>>>        Signed-off-by: softworkz softworkz at hotmail.com
>>>>>>>>
>>>>>>>>  Published-As:
>>>  https://github.com/ffstaging/FFmpeg/releases/tag/pr-
>>>>>>>  ffstaging-72%2Fsoftworkz%2Fsubmit_start_offsets-v1
>>>>>>>>  Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
>>>>>>>  ffstaging-72/softworkz/submit_start_offsets-v1
>>>>>>>>  Pull-Request: https://github.com/ffstaging/FFmpeg/pull/72
>>>>>>>>
>>>>>>>>     libavformat/dump.c | 2 +-
>>>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>
>>>>>>>>  diff --git a/libavformat/dump.c b/libavformat/dump.c
>>>>>>>>  index 8c7db7b275..1bd0424f3d 100644
>>>>>>>>  --- a/libavformat/dump.c
>>>>>>>>  +++ b/libavformat/dump.c
>>>>>>>>  @@ -680,7 +680,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>>>>>>>
>>>>>>>>         if (st->start_time != AV_NOPTS_VALUE && st->start_time !=
>>>  0
>>>>>  &&
>>>>>>>  st->time_base.den && st->time_base.num) {
>>>>>>>>             const double stream_start = av_q2d(st->time_base) *
>>>  st-
>>>>>>>>  start_time;
>>>>>>>>  -        av_log(NULL, AV_LOG_INFO, ", Start-Time %.3fs",
>>>>>>>  stream_start);
>>>>>>>>  +        av_log(NULL, AV_LOG_INFO, ", Start-Time %.6fs",
>>>>>>>  stream_start);
>>>>>>>
>>>>>>>  The camel case is incongruous with the formatting of the text
>>>  next
>>>>>  To
>>>>>>>  It.
>>>>>>  Hi Gyan,
>>>>>>
>>>>>>  as far as I'm seeing it, captions/labels are title case and values
>>>>>  are
>>>>>>  lower case. I would consider "Start-Time" to be a caption/label.
>>>>>  Let's
>>>>>>  look at an example:
>>>>>>
>>>>>>    Stream #0:0[0x8fd]: Video: h264 (High) ([27][0][0][0] / 0x001B),
>>>>>  yuv420p(tv, bt709, top first), 1920x1080 [SAR 1:1 DAR 16:9], Level
>>>  40,
>>>>>  25 fps, 50 tbr, 90k tbn, Start-Time 32476.588s
>>>>>>    Stream #0:1[0x907](dut): Audio: mp2 ([3][0][0][0] / 0x0003),
>>>  48000
>>>>>  Hz, stereo, fltp, 256 kb/s, Start-Time 32476.706s
>>>>>>    Stream #0:2[0x908](dut): Audio: ac3 (AC-3 / 0x332D4341), 48000
>>>  Hz,
>>>>>  5.1(side), fltp, 448 kb/s, Start-Time 32476.643s
>>>>>>    Stream #0:3[0x909](GOS): Audio: mp2 ([3][0][0][0] / 0x0003),
>>>  48000
>>>>>  Hz, stereo, fltp, 256 kb/s, Start-Time 32476.674s (visual impaired)
>>>>>  (descriptions)
>>>>>>    Stream #0:4[0x962](dut): Subtitle: dvb_subtitle ([6][0][0][0] /
>>>>>  0x0006), Start-Time 32476.588s
>>>>>>    Stream #0:5[0x963](dut): Subtitle: dvb_subtitle ([6][0][0][0] /
>>>>>  0x0006), Start-Time 32476.719s (hearing impaired)
>>>>>
>>>>>  I think Gyan means fps, tbr or tbn. These are all lowercase and
>>>>>  abbreviations.
>>>>  Yes, I understood that. But these are units, not labels.
>>>> 
>>>>
>>>>>  The one-line stream info should be as compact as
>>>>>  possible.
>>>>  Agreed.
>>>>
>>>>>  So we should make it shorter, and we should lose the "s" unit as
>>>>>  well.
>>>>  I'm afraid, but omitting the unit makes no sense to me.
>>>>
>>>>  Why?
>>>  Because it is redundant. You always print the start time in seconds,
>>>  users
>>>  will have no problem guessing the time measurement unit of timestamps.
>>>  E.g. see the status line of ffplay or mpv. Timestamps or time
>>>  differences miss the measurement unit, and nobody complianed...
>>  It's redundant only for those who know what it is. Everybody else will
>>  need to guess whether it's s or ms or us and whether it's in the
>>  time base of the stream or not.
>>
>>  With the same argument, you can omit kb/s, fps, tbr, tbn etc.
>>  (user can infer the units from the order and count of values)
>>
>>  This is something that I totally hate when applications are doing it,
>>  and I need to dig down deep into the source code for getting a
>>  definitive answer. Letting users "guess" is not ok IMO.
>> 
>>
>>>>>  Something like:
>>>>>
>>>>>  "in 1234.567"
>>>>  What means "in"?
>>>  in is short for inpoint.
>>>
>>>>  Maybe "offset" if it's gotta be short..?
>>>  Or simply "start", if you don't like "in".
>>  It's not about liking, it's just that "in" doesn't ring any bell for me in
>>  that context.
>>
>>  The problem with start is that it could also mean the timestamp value
>>  where the stream (incl. container) starts, so "start-offset" or
>>  "stream-start" would be more clear imo, even though I wouldn't completely
>>  reject having just "Start".
>> 
>>
>>  Regarding the casing:
>>
>>  Example:
>>  Stream #0:0(eng): Video: h264 (High), yuv420p(progressive), 1920x1080 [SAR
>>  1:1 DAR 16:9], Level 42, 60 fps, 60 tbr, 1k tbn, Start-Time 2.790s
>>  (default)
>>
>>  Why is "Level" capitalized? This fits into my understanding that I
>>  explained before (Labels are capitalized) - how does it fit into the claim
>>  that it should all be lower case?
>
> The formatting and presentation should be consistent. Neither (format) start 
> or duration indicate units so better to keep it that way. If we do change it, 
> it should be for all.
> start-offset wouldn't work since this is printing the absolute first pts of a 
> stream.
> stream-start is not necessary since this is being printed within the line for 
> an individual stream, so 'start' is fine.
> The Case is a nitpick. Not a blocker. It just looked out of place.

Agreed.

Thanks,
Marton