-
-
Notifications
You must be signed in to change notification settings - Fork 360
Update heif_enc.cc #1687
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
Update heif_enc.cc #1687
Conversation
Updated VMT timestamp parser
examples/heif_enc.cc
Outdated
| if (fs.length() == 3) { // ms | ||
| ms = std::stoi(fs); | ||
| } | ||
| else if (fs.length() < 3) { // scale integer up | ||
| uint32_t scale = pow(10.0, 3 - fs.length()); | ||
| ms = std::stoi(fs) * scale; | ||
| } | ||
| else { // scale integer down | ||
| uint32_t scale = pow(10.0, fs.length() - 3); | ||
| ms = std::stoi(fs) / scale; | ||
| } |
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.
You could either use std::pow(10, ... to keep all calculations integer, or use double scale = pow(10.0, ...). Then everything is subsumed in one case instead of three.
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.
Thanks for your feedback @farindk
I've identified a mistake with my millisecond parser and the refactored code will not need to call pow at all or have multiple cases.
Refactored VMT timestamp parser
| heif_raw_sequence_sample_set_duration(sample, ts - *prev_ts); | ||
| heif_track_add_raw_sequence_sample(track, sample); | ||
| } | ||
| else if (ts == *prev_ts) { |
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.
Is it legal input to have similar timestamps following each other?
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.
Yes - for several reasons, including:
- two VMT cues may share the same start time and have different end times
- two VMT cues that share the same start times and end times may have different cue ids or carry different payloads that are agnostic of each other
- where negative cue times are not supported, a simple workaround is to set those negative values to zero (destructively) which may produce unexpected duplicate times
|
Thanks, I have merged it. |
|
These changes add the following features:
|
Updated VMT timestamp parser