-
Notifications
You must be signed in to change notification settings - Fork 30
Add sample_format
to audio metadata
#557
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
@@ -90,6 +90,7 @@ def test_get_metadata(metadata_getter): | |||
) | |||
assert best_audio_stream_metadata.bit_rate == 128837 | |||
assert best_audio_stream_metadata.codec == "aac" | |||
assert best_audio_stream_metadata.sample_format == "fltp" |
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.
Hard-coded value here and below, this test already has lots of these.
# decoded frames. | ||
self._reference_frames[stream_index] = torch.load( | ||
frames_data_path, weights_only=True | ||
) |
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'll add the corresponding reference frames in #556
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.
Also adding this new asset, not strictly needed for this PR, but still useful to check a format that's not fltp.
It will be needed in #556 anyway. It's from TorchAudio.
Towards #549
This PR adds the
sample_format
field toAudioStreamMetadata
. In terms of values, it'll be whatav_get_sample_fmt_name()
returns, so it's a bit "ffmpeg-like": flt, fltp, s16, s32... Maybe we'll want to expose a more user-friendly string? I'm not sure, either way, we can always re-map as follow-ups.