Improve VideoEncoder test against FFmpeg CLI #1058
Conversation
meta-cla
bot
added
the
CLA Signed
| # There may be additional subtle differences in the encoder. | ||
| percentage = 94 if ffmpeg_version == 6 or format == "avi" else 99 | ||
| # MPEG codec used for avi format does not accept CRF | ||
| percentage = 94 if format == "avi" else 99 |
There was a problem hiding this comment.
I had difficulty returning identical frames for avi with the default codec, mpeg4. We could consider choosing a specific codec for this test to enforce quality, but I think its ok to keep the relaxed exactness check in this case.
test/test_encoders.py
Outdated
| fields=fields, | ||
| ) | ||
| for key in ffmpeg_metadata: | ||
| assert ffmpeg_metadata[key] == encoder_metadata[key] |
There was a problem hiding this comment.
I think we should be able to just assert ffmpeg_metadata == encoder_metadata ?
test/test_encoders.py
Outdated
| # Write tensor to file to check with ffprobe | ||
| with open(encoder_output_path, "wb") as f: | ||
| f.write(encoded_output.numpy().tobytes()) |
There was a problem hiding this comment.
I think it's OK to only validate to_file against ffprobe, that's what we do for the audio encoder and it makes this test a tiny bit easier to follow. No strong opinion. If you remove it from here, make sure to remove it from to_file_like as well.
torchcodec/test/test_encoders.py
Lines 353 to 354 in 22bcf4d
2 participants
This PR resolves the first two bullet points in #1057:
test_video_encoder_against_ffmpeg_cliover the other encoding methods (to_file_like,to_tensor)self._get_video_metadatain this test to ensure the expected metadata is the same