Fallback to DTS if PTS info doesn't exist #683
Merged
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
facebook-github-bot
added
the
CLA Signed
NicolasHug
commented
May 15, 2025
Comment on lines
+145
to
+147
| # Should never happen, but prevents ZeroDivisionError: | ||
| or self.end_stream_seconds_from_content | ||
| == self.begin_stream_seconds_from_content |
There was a problem hiding this comment.
This isn't actually needed anymore, i.e. the dts fallback already fixed the metadata issue. But it's probably safer to have this regardless?
There was a problem hiding this comment.
Yeah, let's keep it. I think a single frame video is allowed, and we would encounter this problem with such a video.
scotts
reviewed
May 15, 2025
| # https://huggingface.co/datasets/raushan-testing-hf/videos-test/blob/main/sample_video_2.avi | ||
| # We can't check it into the repo due to potential licensing issues, so | ||
| # we have to unconditionally skip this test.# | ||
| # TODO: encode a video with no pts values to unskip this test. Couldn't |
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.
Some videos, like https://huggingface.co/datasets/raushan-testing-hf/videos-test/blob/main/sample_video_2.avi mentioned in #673, do not have any PTS value for their packets.
Up until now, this caused various weird issues like #677 and #676, and some silently-wrong error (e.g. index-based API with exact mode were always returning the first frame).
This PR allows for proper decoding of such poorly-encoded videos, by falling back to the DTS info if the PTS do not exist.
I added a test, which is skipped unconditionally. We should enable it back once we can check in such a poorly-encoded video. I made sure the test properly pass on https://huggingface.co/datasets/raushan-testing-hf/videos-test/blob/main/sample_video_2.avi with FFmpeg 4 and 6.
(needs #682 first. Note that the
sample_video_2.avivideo is one of those videos were timebase.den != 1, which exposed the bug that is fixed in #682).