bpo-32990: Support WAVE_FORMAT_EXTENSIBLE in the wave module#9515
bpo-32990: Support WAVE_FORMAT_EXTENSIBLE in the wave module#9515ZackerySpytz wants to merge 1 commit intopython:mainfrom
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.
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.
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.
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