-
Notifications
You must be signed in to change notification settings - Fork 67
[NOT_READY] Ensure OTIO video triming is duration accurate (for re-encoding to avoid keyframe rounding issue). #1394
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
Draft
rdelillo
wants to merge
1
commit into
develop
Choose a base branch
from
bugfix/AY-8039_video_trim_duration_mismatch
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+0
−1
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
@jakubjezek001 @iLLiCiTiT @philippe-ynput @antirotor
I'm torn by this change. Using the
-c copystream option when trimming the video get us codec accurate sub-videos BUT with potential duration mismatch due to keyframes (see PR description).While removing this as I do here, force a re-encoding, giving frame-accurate sub-videos but through different codecs than source (ffmpeg default is x264) which might not be what we want here.
I could go with a more sophisticated approach is and detect codec from source than attempt to re-encode in same codec but not all codec variations are natively supported by FFmpeg and we could still end-up with subtle variations (color, gamma, bitrate...) compare to
copy.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.
I guess the codec copy is important.
There is
get_ffmpeg_codec_argsto do so.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.
The accuracy of the cut is probably more important than the image quality, assuming the re-encoding will not totally destroy the image quality.
Uh oh!
There was an error while loading. Please reload this page.
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.
Just gave it more testing this afternoon this is not good enough.
I have to set
copystream option to ensure frame accurate (but not duration accurate) stuff.When I re-encode blindly from input I got misinterplated interlaced-frames.
The only correct way I have to to "trim" properly the video right now is to extract the video as image sequence than re-encode the portion required in order to leave no room for video codec subtilities. But this is obviously way slower.
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.
Why dont we just simply convert the trimmed cut of the video to intermediate PNG image sequence? It would secure that we are on frame accurate and also would preserve the most of the video.
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.
Definitely we simply wouldn't keep the video native encoding/codec metadata.
This could end up looking different on some player.