Conversation
| @@ -129,8 +129,15 @@ def num_frames(self) -> Optional[int]: | |||
| """ | |||
There was a problem hiding this comment.
In the doc string, we should also say what we do if num_frames is missing from all of the metadata.
| elif ( | ||
| self.average_fps_from_header is not None | ||
| and self.duration_seconds_from_header is not None | ||
| ): |
There was a problem hiding this comment.
I just went down an interesting rabbit-hole of asking "Should we instead use the average_fps property?" And the answer is no! :) If self.average_fps_from_header is None, then self.average_fps actually calls this property, self.num_frames and we'd (I think) hit infinite recursion. I do wonder if there is a way for us to be more systematic about this, where we could more centrally define the logic of under what circumstances we use which metadata.
| all_frames = decoder.get_frames_played_in_range( | ||
| decoder.metadata.begin_stream_seconds, decoder.metadata.end_stream_seconds | ||
| ) | ||
| assert_frames_equal(all_frames.data, decoder[:]) |
There was a problem hiding this comment.
Can we also add a simple assert len(decoder) == CORRECT_VALUE?
Thank you! Also, this is only half of #727. We also want to be able to handle if |
This PR enables the instantiation of a VideoDecoder without
num_frames_from_contentornum_frames_from_headerin the metadata, to resolve issue #727. Without these variables, the VideoDecoder will calculate the number of frames usingaverage_fps_from_headerandduration_seconds_from_header.Tests in test_decoders.py
_get_stream_json_metadatais mocked to ensure VideoDecoder is instantiated withoutnum_frames_from_contentornum_frames_from_header.get_frame...methods are tested on this decoder.Tests in test_metadata.py
test_calculate_num_frames_using_fps_and_durationinstantiatesVideoStreamMetadatawithoutnum_frames_from_contentornum_frames_from_header, and checks that various values ofaverage_fpsandduration_secondsresult in the correct number of frames.In
test_num_frames_fallback, the test where bothnum_frames_from_contentandnum_frames_from_headerareNoneis no longer valid. A similar test case is added intest_calculate_num_frames_using_fps_and_duration.