Calculate num_frames if missing from metadata

[Dan-Flores]

This PR enables the instantiation of a VideoDecoder without certain metadata to resolve issue #727.

• Without num_frames_from_content or num_frames_from_header in the metadata, the VideoDecoder will calculate the number of frames using average_fps_from_header and duration_seconds_from_header.
• Without duration_seconds_from_header, or end_stream_seconds_from_content and begin_stream_seconds_from_content from a scan, the VideoDecoder will calculate the duration using num_frames_from_header and average_fps_from_header.

Tests in test_decoders.py

• The return value of _get_stream_json_metadata is mocked to ensure VideoDecoder is instantiated without num_frames_from_content or num_frames_from_header.
• Python logic evaluating the metadata in VideoDecoder instantiation and get_frames_in_range are tested.

Tests in test_metadata.py

• test_calculate_num_frames_using_fps_and_duration instantiates VideoStreamMetadata without num_frames_from_content or num_frames_from_header, and checks that various values of average_fps and duration_seconds result in the correct number of frames.

• In test_num_frames_fallback, the test where both num_frames_from_content and num_frames_from_header are None is no longer valid. A similar test case is added in test_calculate_num_frames_using_fps_and_duration.

• test_duration_seconds_fallback validates that calculating metadata using a scan is prioritized over header metadata.

• test_calculate_duration_seconds_using_fps_and_num_frames tests the fallback implemented in this PR.

[scotts]

In the doc string, we should also say what we do if num_frames is missing from all of the metadata.

[scotts]

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.

[scotts]

Can we also add a simple assert len(decoder) == CORRECT_VALUE?

[scotts]

I had initially approved, but it looks like the tests are failing on Mac - let's make sure we're using the right tolerances there.

[scotts]

Thank you! Also, this is only half of #727. We also want to be able to handle if num_frames is present but duration is not. This half, though, is definitely the more valuable side, as core operations in the Python VideoDecoder depend on num_frames.

[scotts]

Let's add a comment here that we're really just testing the logic in the Python VideoDecoder class when we do the decoding. We do checks that involve num_frames at the Python layer, and that logic will use the mocked metadata. But the C++ layer does not use that mocked metadata, and will instead use what it read from the video file itself.

We should also create an issue about adding a video file that has the number of frames missing in its metadata so we can actually test the C++ logic as well. This test would then use that video file, and not need any mocking.

[Dan-Flores]

Good point! I'll remove some unnecessary python calls here, and add a comment indicating that the python logic is the only aspect being tested.

It looks like issue #710 covers this subject.

[scotts]

Thanks, @Dan-Flores! I looked into the failures, and they're all known (#740, #734, #724).

[Dan-Flores]

I've updated this PR to complete the other half of #727, and updated the description as well.