-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use VideoTimestamps #21
base: master
Are you sure you want to change the base?
Use VideoTimestamps #21
Conversation
continue | ||
chapters.append((time, f"Chapter {i:02.0f}")) | ||
if chapters and _print: | ||
for time, name in chapters: | ||
print(f"{name}: {format_timedelta(time)} | {timedelta_to_frame(time, fps)}") | ||
print(f"{name}: {format_timedelta(time)} | {ms_to_frame(int(time.total_seconds() * 1000), TimeType.EXACT, fps)}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong timetype
print(f"{name}: {format_timedelta(time)} | {ms_to_frame(int(time.total_seconds() * 1000), TimeType.EXACT, fps)}") | |
print(f"{name}: {format_timedelta(time)} | {ms_to_frame(int(time.total_seconds() * 1000), TimeType.START, fps)}") |
I realized that the VideoTimestamps performs all its calculations in milliseconds. That's perfect for SRT, WebVTT, ASS, and any format that has a maximum precision in milliseconds. However, for any format that exceeds millisecond precision, it won't be perfect when the file is not an MKV. It will work well with MKV files because their precision is (99% of the time) in milliseconds. So, if we have a time in nanoseconds, there is a way to properly handle it in milliseconds. But for non-MKV files (e.g., M2TS), the precision can be "infinite," making it impossible to properly convert a time, let's say in nanoseconds, to milliseconds. However, I talked with cubicibo (who has the Blu-ray spec), and he told me that a chapter needs to be exactly the frame time (a.k.a. As for subtitles, they should technically never exceed millisecond precision since ASS, WebVTT, SRT, etc., cannot represent those values. Therefore, if we truncate the timedelta, we don't lose any information. If you ever need a def timedelta_to_ms(
time: timedelta, time_type: TimeType, rounding_method: RoundingMethod = RoundingMethod.ROUND
) -> int:
"""
Converts a timedelta to milliseconds without losing information.
:param time: The timedelta.
:param time_type: The time type.
:param rounding_method: If you want to be compatible with mkv, use RoundingMethod.ROUND else RoundingMethod.FLOOR.
For more information, see the documentation of [timestamps](https://github.com/moi15moi/VideoTimestamps/blob/683a8b48ad394d60ced0deda0ddb87b70e0bfa83/video_timestamps/timestamps.py#L14-L29)
:return: The resulting frame number.
"""
if rounding_method == RoundingMethod.ROUND:
time_ms = time.total_seconds() * 1000 # total_seconds returns a float, which can have imprecision.
if time_type in (TimeType.START, TimeType.END):
ms = ceil(time_ms)
elif time_type == TimeType.EXACT:
ms = floor(time_ms)
else:
raise ValueError(f"The TimeType {time_type} isn't supported.")
elif rounding_method == RoundingMethod.FLOOR:
# It is impossible to ensure precision for the FLOOR method because it is impossible
# to convert, for example, nanoseconds to milliseconds without losing information.
# It works well for the ROUND method because the maximum precision of video timestamps
# is in milliseconds, so we don't lose any information in the conversion process.
# Note: For chapters, the specification says that it should always be TimeType.EXACT,
# so we can simply floor it.
raise ValueError("It is impossible to ensure precision for the FLOOR method.")
else:
raise ValueError(f"The RoundingMethod {rounding_method} isn't supported.")
return ms Edit (08-18-2024): In the BD spec (thanks again cubicibo), for a Exemple: In nanoseconds, it gives: If we compare it with the equation we use to approximate the time with an |
A new version of VideoTimestamps is available. |
590809a
to
2c45566
Compare
2c45566
to
8c18119
Compare
@Vodes I just realised that the structure of muxtools doesn't allow to know what is the video timescale (it is the inverse of time_base). It is a requirement to create FPSTimestamps or TextFileTimestamps objects. |
I don't quite understand what that timescale is supposed to be. As for timedeltas: |
To understand what a Even if the ExampleFor mkv file, the And then, from what, we can find the time: Note that for .m2ts file, the timescale is always 90000. If you calculate if, you will see that it doesn't give exactly the same time has with the timescale 1000 which is why you need to precise it when you create a FPSTimestamps or TextFileTimestamps objects |
Someone asked my in pm why I opened a PR. Here is a test that compare muxtools converter and VideoTimestamps one: import os
from datetime import timedelta
from fractions import Fraction
from pathlib import Path
from muxtools.utils.convert import timedelta_to_frame, frame_to_timedelta, frame_to_ms
from video_timestamps import FPSTimestamps, RoundingMethod, TimeType, TextFileTimestamps
dir_path = Path(os.path.dirname(os.path.realpath(__file__)))
def test_frame_to_ms():
fps = Fraction(24000, 1001)
timestamps = FPSTimestamps(RoundingMethod.ROUND, Fraction(1000), fps)
frame = 18
assert frame_to_ms(frame, fps) == timestamps.frame_to_time(frame, TimeType.EXACT, 3)
def test_timedelta_to_frame():
fps = Fraction(24000, 1001)
timestamps = FPSTimestamps(RoundingMethod.ROUND, Fraction(1000), fps)
ms = 124
assert timedelta_to_frame(timedelta(milliseconds=ms), fps) == timestamps.time_to_frame(ms, TimeType.EXACT, 3)
def test_with_timestamps_file():
fps = dir_path.joinpath("timestamps.txt")
timestamps = TextFileTimestamps(fps, Fraction(1000), RoundingMethod.ROUND)
ms = 501
assert timedelta_to_frame(timedelta(milliseconds=ms), fps) == timestamps.time_to_frame(ms, TimeType.EXACT, 3) Here is what
Here is a zip file (proof.zip) with the py and txt files. All the tests currently fails. |
So... From what I gather you just want a way to provide the timescale thingy and it would already solve a lot of problems? |
Yes, if you are able to have the timescale everywhere where you call timedelta_to_frame, frame_to_timedelta and frame_to_ms, we could use VideoTimestamps which would resolve all the problem. |
Okay I'll try to think of something on the weekend Edit: I caught a pretty bad flu on Thursday so this may be delayed |
The code you posted produces 0 for 42 ms in |
Meanwhile, in |
My bad, I didn't write the tests correctly. I edited my message to fix it.
Are you sure you are on muxtools master branch? |
Indeed, this seems to have fixed 42 ms. Thanks. But the new test uses a timestamp beyond the end of the file. video_timestamps seems to extrapolate the timestamps endlessly (e. g. 601 gives 14), but I don’t see how this case is relevant or correct, given that no timestamps beyond the end of the file are known.
You’re right; I wasn’t. But the difference is explicitly intentional:
(This says 0.01, but the actual threshold was raised to 0.03 in d72c8a1.) |
To be clear, with Finally, (As the test is written, this is more precise than the whole-millisecond timestamp returned by |
VideoTimestamps try to extrapolate the time like aegisub does.
|
As for video timebase/scale, I’m confused because:
I don’t know what goals VideoTimestamps has and why. In this PR, you’re suggesting a muxtools change, and it seems muxtools already handles timestamp files just fine.
You know, all your examples just show that the behaviour of muxtools is different from VideoTimestamps, with a very specific configuration at that, but they do nothing to explain why VideoTimestamps results are better—to demonstrate the actual problem you’re trying to solve—which would be more useful to see. Like, why do you think 83 is supposed to return 2 and 124 is also supposed to return 2? I’ll try to infer from your examples. Are you asserting that the default FPS handling in muxtools targets typically-rounded integer-millisecond Matroska timestamps as a common use case, and the current calculations don’t match this specific target? For example, 2 frames at 24M fps are about 83.417 ms, which gets rounded to 83, so you expect 83 to map to 2 because that’s what it would be mapped to in a standard MKVToolNix-produced millisecond-precision Matroska file. And 3 frames are (exactly) 125.125 ms, which gets rounded to 125, so you expect 125 to map to 3 and 124 to 2, because in the same Matroska file 124 would still be within frame 2. Is this correct? |
How do you expect |
Oh, and if my guess about your assumptions is correct, then I think the timebase thing you mentioned is just that this currently-assumed millisecond precision should be configurable? Is this right? |
If you want to convert a time to a frame or vice-versa, you necessarily have a video or a idea of how the video would be (like, you know the fps of your video, so with this logic, you also know the timescale of your video).
Yes, you are totally right when you say A timestamps file may be rounded by mkvmerge depending on the timescale you supply with --timestamp-scale factor (warning, timestamps-scale doesn't directly mean timescale, but there is a way to convert from one to the other), so it is not true that a timestamp file represent perfectly the PTS what will be generated by mkvmerge.
Yes, in general, the timescale for mkv is 1000, so the maximum precision about the PTS are in milliseconds. But, for example, for m2ts file, the timescale is 90000, so there isn't a decimal form available.
I don't have a example with a Here is a updated proof.zip with the fps 30 (also I changed the TimeType.EXACT to TimeType.START in the test because it seems that muxtools want this behaviour).
Yes, but note that any other use case like m2ts file currently aren't properly supported. You cannot ignore the timescale.
You cannot calculate like this. Please, read this message.
Depending on the TimeType, it will floor, round or ceil.
Yes, we could say that. By default, the timescale parameter could be 1000 (which is milliseconds). |
I just did calculate exactly how your message describes. |
I tought you rounded the time, not the PTS.
It depend on which TimeType you are talking about. EXACT START END |
With a time base of 1 ms, PTS is time, expressed in milliseconds.
This makes sense to me. But weren’t we talking about |
Is has one, it is just not properly stated. It seems to be TimeType.EXACT. |
Ah, it looks like you’re adding IIUC, this should be good for frame rates up to 1000 fps, which is probably mostly sufficient for muxtools. Matroska chapters have nanosecond precision, so they would benefit from And subtitles might possibly benefit from I suggest that both of these be investigated after dealing with the current stuff. Chapters need other fixes anyway (e. g. to stop rounding them to centiseconds for a start). |
In my opinion, a |
I replaced some functions in
convert.py
to use VideoTimestamps. Now, the functionsms_to_frame
andframe_to_ms
are frame-perfect.There are breaking changes:
frame_to_timedelta
has been replaced withframe_to_ms
.timedelta_to_frame
has been replaced withms_to_frame
.This PR will require extensive testing.