Skip to content

Conversation

@gwenael-hagenmuller
Copy link

This fix can be considered as being a breaking change since the results
will be different and that could impact consumers of this library.

The changes were done only for the case where there is no dropped frame.
allow_inaccurate was added to parse_with_fractional_seconds,
parse_with_ticks and parse with the default value value so those 2
last functions (not parse) are backward compatible.

Also fixed the examples shared in the documentation. Even before the
changes of this commit, the 2nd example was wrong. The issues were

You are calculating timecodes with different drop flag values (Timecode::WrongDropFlag)
You are calculating timecodes with different framerates (Timecode::WrongFramerate)

The number of frames for the timecode `15:25:13:01`
at `23.976` fps (`24000/1001` to be precise) is `1330982.018981019`:
```
(15 * 3600 + 25 * 60 + 13) * 24000.0/1001.0 + 1
```
not `1332313`.

The next commit will fix this.
This fix can be considered as being a breaking change since the results
will be different and that could impact consumers of this library.

The changes were done only for the case where there is no dropped frame.
`allow_inaccurate` was added to `parse_with_fractional_seconds`,
`parse_with_ticks` and `parse` with the default value `value` so those 2
last functions (not `parse`) are backward compatible.

Also fixed the examples shared in the documentation. Even before the
changes of this commit, the 2nd example was wrong. The issues were
```
You are calculating timecodes with different drop flag values (Timecode::WrongDropFlag)
You are calculating timecodes with different framerates (Timecode::WrongFramerate)
```
@gwenael-hagenmuller
Copy link
Author

Thanks for your gem. I noticed it didn't handle non integer fps adequately so I went ahead and fix that. Please review commit per commit.

@gwenael-hagenmuller gwenael-hagenmuller changed the title fix!: handle non integer frame rates adequatly fix!: handle non integer frame rates adequately Mar 7, 2025
@julik
Copy link
Member

julik commented Jun 1, 2025

Sorry for reacting so late. The gem was designed to handle different framerates and calculate timecodes, but I don't think letting it return a Float for the frame offset is a good choice. I made it back in the day so that every frame could be identified with a non-ambiguous counter - the start offset of that counter is a different matter, of course. If working with fields I could imagine allowing U/F for sub-frame offsets can be doable, but that's only 2 values (the 0.0 and 0.5 - and that would be better to represent with an additional Symbol or something, so that no rounding gets done to these).

With regards to a9f0ca2 - is there a system which actually produces this timecode with this framerate? I can understand this being useful in an audio system where you need subframe accuracy, but for images / image sequences it seems like this feature will be confusing. If a timecode at a framerate is not referencing a particular whole frame - I prefer the library to barf and say it, instead of inventing a subframe offset.

Can you explain the rationale a bit more?

@julik julik self-requested a review June 1, 2025 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants