Repository navigation
Fix S3 root-level object discovery, document remote file_root, add S3 video tests - #4
Closed
jantonguirao wants to merge 5 commits into
Closed
jantonguirao wants to merge 5 commits into
jantonguirao wants to merge 5 commits into
Conversation
Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
…:// bucket. Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
…_root s3_discover_files asserted that every listed object sits at least one subdirectory below the root. For an object directly under the root this aborts in debug builds and dereferences end() in release builds. Skip such objects instead, as the local and GCS discovery do. The video reader makes this case reachable for s3:// file_root. Mention remote URLs in the video reader's file_root documentation. Signed-off-by: Joaquin Anton Guirao <janton@nvidia.com>
Cover the filenames, file_list and file_root arguments on s3:// URLs, the latter with a video directly under the root. The last video of each device is larger than the reader's I/O buffer, so it needs several ranged reads. Signed-off-by: Joaquin Anton Guirao <janton@nvidia.com>
mzient
force-pushed
the
gcs_s3_video_reader
branch
2 times, most recently
from
October 29, 2026 08:41
61fa9ce to
6b7c5d9
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
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.
Category:
Bug fix
Description:
Follow-up to NVIDIA#6518, addressing the review comments there.
s3_discover_filesasserted that every listed object has at least two path elements. For an object directly underfile_rootthis aborts in debug builds and dereferencesend()in release builds. The video reader makes this case reachable fors3://roots, so such objects are now skipped, as the local and GCS discovery do.file_rootargument of the video reader now mentions remotes3://andgs://URLs, likefilenamesandfile_listalready do.filenames,file_list,file_root, on CPU and GPU). Thefile_rootlayout includes a video directly under the root, and the last video of each device is larger than the reader's 1 MiB I/O buffer, so it needs several ranged reads.Additional information:
Affected modules and functionalities:
dali/operators/reader/loader/discover_files_s3.cc: skip objects with fewer than two path elements instead of asserting.dali/operators/video/reader/video_reader_decoder_op.cc:file_rootdocs.dali/test/python/reader/test_s3.py: video fixtures and tests.Key points relevant for the review:
discover_files_s3.ccchange also affectsfn.readers.fileon S3 for roots with objects directly under them.test_s3.pyare copied fromtest_gcs.py. They could be moved to a shared module if you prefer.Tests:
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A