fix: timeRange trimming drops the first timeRange.start of the selection. - #60
Open
madsodgaard wants to merge 1 commit into
Open
fix: timeRange trimming drops the first timeRange.start of the selection.#60madsodgaard wants to merge 1 commit into
timeRange trimming drops the first timeRange.start of the selection.#60madsodgaard wants to merge 1 commit into
Conversation
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
startSession(atSourceTime: timeRange.start)already makes AVAssetWriter rebasesource time to movie time, but video frames are also appended at
PTS - timeRange.start. That applies the offset twice, so frames beforetimeRange.startresolve to a negative movie time and the writer discards them —appendstill returnstrue, so nothing surfaces as an error. Audio is appendedunmodified and keeps the full range.
Trimming an 18s clip to 3s–18s gave me a 15s file with a 12s video track and a
15s audio track: playback starts ~3s into the selection, and the final frame is
frozen for the remaining 3s.
Appending with the original presentation timestamp fixes both append paths and
matches what the audio path already does. Progress reporting still uses the
range-relative time, so that behaviour is unchanged.
One thing worth knowing: this dates back to the initial commit, but it used to be
masked.
CVPixelBufferPoolCreatePixelBufferfails withkCVReturnInvalidPixelFormat(the pool attributes ask forkCVPixelFormatType_32RGBA), sohandledstayedfalseand every frame fellthrough to the plain
input.append(sampleBuffer), which got the timing right byaccident. #39's black-frame fix skips that allocation when there's no render
handler, which is what made the bug reachable in 1.0.1.