-
-
Notifications
You must be signed in to change notification settings - Fork 33.4k
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