-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
bpo-32990: Support WAVE_FORMAT_EXTENSIBLE in the wave module #9515
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
bpo-32990: Support WAVE_FORMAT_EXTENSIBLE in the wave module #9515
Conversation
The test file, a modified version of Lib/test/audiodata/pluck-pcm24.wav, was provided by Andrea Celletti on the bug tracker.
482bb86
to
80b360a
Compare
if wFormatTag == WAVE_FORMAT_EXTENSIBLE: | ||
# Only the first 2 bytes (of 16) of SubFormat are needed. | ||
cbSize, wValidBitsPerSample, dwChannelMask, \ | ||
SubFormatFmt = struct.unpack_from('<HHLH', chunk.read(10)) |
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.
Could you reformat this line in a way that makes it easier to read? Right now it looks like two separate statements.
Using parentheses around the left hand side is fine, and then indent the second line.
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.
Oh, we should also catch struct.error
here and raise, like we do above.
wFormatTag, self._nchannels, self._framerate, dwAvgBytesPerSec, wBlockAlign = struct.unpack_from('<HHLLH', chunk.read(14)) | ||
wFormatTag, self._nchannels, self._framerate, dwAvgBytesPerSec, \ | ||
wBlockAlign, wBitsPerSample = \ | ||
struct.unpack_from('<HHLLHH', chunk.read(16)) |
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.
Previously if there was no wBitsPerSample
value and wFormatTag
was something other than WAVE_FORMAT_PCM
, we may get a different error.
Can we be sure that the longer chunk.read
will always return 16 bytes? Or should we keep deferring the second read until after we've checked wFormatTag
FYI, we are planning to deprecate the wave module in 3.8 and move it out of the stdlib in 3.9 or 3.10. Does it make sense to add new features at all? |
Has there been a final decision on this? It seems the plan is to keep |
@ZackerySpytz, please address the code review. Thank you! |
I'm in the same boat. Any objections to starting a new PR? |
I will update this PR. A new one doesn't need to be created. |
python/cpython#9515 only adds support for reading WAVE_FORMAT_EXTENSIBLE, not writing.
Same change was merged in #96777 |
The test file, a modified version of
Lib/test/audiodata/pluck-pcm24.wav, was provided by Andrea Celletti
on the bug tracker.
https://bugs.python.org/issue32990