On Tue, 22 Apr 2025, Gyan Doshi wrote:



On 2025-04-22 02:22 am, softworkz . wrote:

 -----Original Message-----
 From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
 Marton Balint
 Sent: Montag, 21. April 2025 22:18
 To: FFmpeg development discussions and patches <ffmpeg-
 de...@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-boun...@ffmpeg.org> On Behalf Of
 Marton Balint
 Sent: Montag, 21. April 2025 21:32
 To: FFmpeg development discussions and patches <ffmpeg-
 de...@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-boun...@ffmpeg.org> On Behalf Of
 Gyan
 Doshi
 Sent: Montag, 21. April 2025 06:51
 To: ffmpeg-devel@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 <softwo...@hotmail.com>

 Changing this to 6 digits to align with other
 printed times

 Signed-off-by: softworkz <softwo...@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 softwo...@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to