-
Notifications
You must be signed in to change notification settings - Fork 77
Check in json files with test frame infos #541
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
|
|
||
| # {stream_index -> {frame_index -> frame_info}} | ||
| default_stream_index: int | ||
| stream_infos: Dict[int, Union[TestVideoStreamInfo, TestAudioStreamInfo]] |
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.
Moving that field into the base class TestContainerFile which will be needed for audio anyway.
It needed here already because we need to know the number of streams within __post_init__ just below, and we using stream_infos for that.
(Ideally we wouldn't need to, and just collapse all classes to be single-stream, but that's for later).
| # .frames may be manually set: for some streams, we don't need | ||
| # the info for all frames. We don't need to load anything in | ||
| # this case | ||
| continue |
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 don't feel too strongly about this but if we were to enforce the existence of a json file for every single asset, it would raise the barrier to the first test when e.g. adding a new test asset, which I think isn't desirable.
| 35: TestFrameInfo(pts_seconds=1.1678, duration_seconds=0.033367), | ||
| }, | ||
| }, | ||
| frames={}, # Automatically loaded from json file |
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.
We have to create an empty frames field here. This field cannot have a default within the TestContainerFile definition, because defaults aren't allowed for base dataclasses
No description provided.