Scan every followed video stream in one pass, and only before demuxing - #1678
Merged
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/meta-pytorch/torchcodec/1678
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 7 PendingAs of commit 1608594 with merge base ba6b3f7 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
NicolasHug
force-pushed
the
msd/cpp-cached-scan
branch
2 times, most recently
from
September 2, 2026 11:06
3361e63 to
e1b8c56
Compare
NicolasHug
force-pushed
the
msd/cpp-cached-scan
branch
from
September 2, 2026 12:52
e1b8c56 to
d394b1e
Compare
NicolasHug
force-pushed
the
msd/cpp-cached-scan
branch
from
September 2, 2026 12:52
e1b8c56 to
d394b1e
Compare
NicolasHug
force-pushed
the
msd/cpp-cached-scan
branch
from
September 2, 2026 13:41
d394b1e to
d591993
Compare
NicolasHug
force-pushed
the
msd/cpp-cached-scan
branch
2 times, most recently
from
September 2, 2026 13:47
d394b1e to
c19d13b
Compare
NicolasHug
force-pushed
the
msd/cpp-cached-scan
branch
from
September 2, 2026 13:51
c19d13b to
2111c09
Compare
NicolasHug
commented
Sep 3, 2026
| ReferenceAVPacket packet(auto_packet_); | ||
| int status = | ||
| read_next_packet(format_context_.get(), scanned_stream->index, packet); | ||
| read_next_packet(format_context_.get(), video_stream_indices, packet); |
Contributor
Author
There was a problem hiding this comment.
Do we still need the read_next_packet that only takes a single stream index then??
Contributor
Author
There was a problem hiding this comment.
Yes, it's used in SingleStreamDecoder
NicolasHug
force-pushed
the
msd/cpp-cached-scan
branch
2 times, most recently
from
September 3, 2026 08:29
ccd7717 to
fda056c
Compare
NicolasHug
force-pushed
the
msd/cpp-cached-scan
branch
2 times, most recently
from
September 3, 2026 08:33
2aa0235 to
4792ecd
Compare
A scan used to read the container once per stream and cache nothing, so indexing two video streams read the file twice and calling scan() twice read it twice again. It is now two phases: one pass over the container buckets the packets of every followed video stream, and the sort plus tensor build happens per stream, the first time that stream's index is asked for. The I/O is what a scan costs, and it is now paid once. Scanning after the first packet has been demuxed raises. The scan rewinds the container, which would silently desynchronise any decoder already being fed from it - and with several streams there are several decoders to remember to reset. Requiring the scan up front removes the problem instead of documenting it: nothing has been fed to a decoder yet, so there is nothing to reset, and the "a decoder built from it must be reset()" caveat is gone. Scanning a non-video stream now raises rather than quietly producing a frame index for something that has no frames or keyframes. A test pins the rewind behaviour on discard_first_keyframe.mp4: the scan rewinds with a seek rather than reopening, and on a file whose edit list discards its first packets those are not obviously equivalent. They are.
NicolasHug
force-pushed
the
msd/cpp-cached-scan
branch
from
September 3, 2026 08:59
4792ecd to
1608594
Compare
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
1 participant
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.
A scan used to read the container once per stream and cache nothing, so indexing two video streams read the file twice and calling scan() twice read it twice again. It is now two phases: one pass over the container buckets the packets of every followed video stream, and the sort plus tensor build happens per stream, the first time that stream's scan is asked for. The I/O is what a scan costs, and it is now paid once. The sorting is paid per-stream (no way around that), but only if / when requested.
Scanning after the first packet has been demuxed raises.
Scanning a non-video stream now raises rather than quietly producing a frame index for something that has no frames or keyframes.
Stack created with GitHub Stacks CLI • Give Feedback 💬