-
Notifications
You must be signed in to change notification settings - Fork 71
Refactor Frame and FrameBatch into their own source file #247
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
Conversation
|
@scotts has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
NicolasHug
left a comment
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.
LGTM with one small request to move it one level up
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.
Can you actually move this file one level up into src/torchcodec/? We'll need that to expose them in torchcodec as discussed in #221 (comment)
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 remembered that discussion, but got the conclusion wrong. :)
|
@scotts has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@scotts has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@scotts has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@scotts has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
This will simplify my future PRs where we start implementing
SingleStreamDecoder.