Re: [FFmpeg-devel] [PATCH] libavutil/timestamp.h: Fix loss of precision in timestamps for silencedetect on long files

[Allan Cady via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

[Apologies for the awful mess in the previous email. Trying again with straight text.]

On Thursday, February 22, 2024 at 01:16:19 AM PST, Marton Balint <cus@passwd.hu> wrote:

>> For starters, I'm curious why there are two functions & macros:
>>
>> av_ts2str/av_ts_make_string (which used "%" format specifier)
> 
> That takes a 64-bit integer timestamp and is actually using "%"PRId64 
> because that is the correct (portable) format string for an int64_t 
> variable.
> 
>> av_ts2timestr/av_ts_make_time_string (which used "%6g")
> 
> That takes an integer timestamp and a rational time base. Float timestamps 
> (in seconds) is calculated by multiplying the two, that is what is 
> printed.
> 
>>
>> Do you know the rationale for that? I see that only av_ts2timestr is used in silencedetect.c.
>>
>> And are you suggesting we should fold those two functions into one?
> 
> No, they have different purpose. The first prints out a timestamps which 
> can be in any time base. The second prints out a timestamp in seconds.


Understood, I think I'm up to speed now. av_ts2str prints a 64-bit integer timestamp;
av_ts2timestr prints a floating point timestamp in seconds with the timebase applied.


In your previous email, you said:


> I'd rather just to fix av_ts_make_string to not limit the number of 
> significant digits. Something like:
>
> 1) Print the number in decimal notation with at most 6 fractional digits. 
> 2) Use less fractional digits if the first format would not fit into 
> AV_TS_MAX_STRING_SIZE.
> 3) Use scientific notation if the second format would not fit into 
> AV_TS_MAX_STRINT_SIZE.


I think you probably meant to say "I'd rather just to fix
av_ts_make_time_string" (not av_ts_make_string)?
Since it's av_ts_make_time_string() that's formatting floating point.


So it makes sense to me to make the change to av_ts_make_time_string()
for all timestamps, as you suggest. As for how specifically to format them,
I'm fine with whatever you think is best, and I'm happy to work on this, but the
implementation has me a bit stumped for the moment, and I may need some
help with it. My C language skills are rusty to say the least.

It also occurs to me to wonder, would this warrant a formal problem ticket?
I haven't looked into how that works for ffmpeg.

Thanks for your help.




_______________________________________________
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".