Hi all,
while revisiting this patch, I am still not 100% sure whether I broke something with sounds buffer for BigEndian systems.
Are there any current VM implementations for BigEndian systems? If yes, could anyone who reads this please be so nice and test out whether this snippet works correctly on your system (i.e., plays a normal sound)?
| array | array := ByteArray streamContents: [:stream | PluckedSound bachFugue storeWAVSamplesOn: stream]. (FileStream fileNamed: 'bachFugue.wav') binary in: [:stream | [array do: [:ea | stream nextPut: ea]] ensure: [stream close]]. (SampledSound fromWaveFileNamed: 'bachFugue.wav') play.
Otherwise, I will treat this issue as resolved for new. Thanks in advance! :-)
Best, Christoph
--- Sent from Squeak Inbox Talk
On 2020-09-02T21:35:50+00:00, christoph.thiede@student.hpi.uni-potsdam.de wrote:
Hi David, thanks for the explanation!
Given that explanation, the patched method will still not work correctly for ByteArray on a BigEndian system, is that correct? (I do not have a BigEndian system available.) Should we define reverseBytes as the following instead?
reverseBytes := streamDirect ifTrue: [bigEndianFlag xor: Smalltalk isBigEndian] ifFalse: [bigEndianFlag].
Best,
Christoph
http://www.hpi.de/ ________________________________ Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von David T. Lewis <lewis at mail.msen.com> Gesendet: Dienstag, 1. September 2020 20:48:55 An: The general-purpose Squeak developers list Betreff: Re: [squeak-dev] The Trunk: Sound-ct.71.mcz
On Tue, Sep 01, 2020 at 06:03:09AM +0000, Thiede, Christoph wrote:
Hi David, hi all, thanks for merging!
Do you know what Smalltalk endianness is good for? Is this maybe only relevant for FileStreams at all?
Endianness is important when you need to know about the memory addressing conventions of the platform that Squeak is running on. For example, the elements of a ByteArray are always accessed consistenty in Smalltalk (e.g the first element is aByteArray at: 1). But the internal storage of the bytes in the object memory is done differently on little-endian versus big-endian platforms.
If you look for senders of #endianness, #isBigEndian, and #isLittleEndian you will find lots of places where the endianness is important.
It is worth noting also that the object memory gets fixed up for endianness when the image is loaded. Thus if you save an image on a little-endian machine, then load and run that image on a big-endian machine, then the byte ordering gets swapped at image loading time. Within Squeak, everything looks the same, but internally in the VM the bytes will be been swapped into the appropriate ordering.
There is a good overview at https://en.wikipedia.org/wiki/Endianness
Dave
Best, Christoph ________________________________ Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von David T. Lewis <lewis at mail.msen.com> Gesendet: Dienstag, 1. September 2020 04:13:16 An: squeak-dev at lists.squeakfoundation.org Cc: packages at lists.squeakfoundation.org Betreff: Re: [squeak-dev] The Trunk: Sound-ct.71.mcz
Thanks Christoph, good catch and thanks for the fix.
I moved this and the supporting test immediately to trunk because it is clearly a worthwhile change.
All - Christoph requests detailed review, please do so and make any updates if appropriate.
I'm really happy to see long-overlooked details like this being addressed :-)
Dave
On Tue, Sep 01, 2020 at 02:01:27AM +0000, commits at source.squeak.org wrote:
David T. Lewis uploaded a new version of Sound to project The Trunk: http://source.squeak.org/trunk/Sound-ct.71.mcz
==================== Summary ====================
Name: Sound-ct.71 Author: ct Time: 1 September 2020, 2:07:15.07999 am UUID: 0d91a1bf-41cb-834c-ab0c-fa2ad832e408 Ancestors: Sound-nice.69
Fixes wave sound streaming on non-filestream objects. The endianness was inverted because #int16: already uses Big Endian. This did not sound well - listen yourself in an unpatched image: :-)
array := ByteArray streamContents: [:stream | PluckedSound bachFugue storeWAVSamplesOn: stream]. (FileStream fileNamed: 'bachFugue.wav') binary in: [:stream | [array do: [:ea | stream nextPut: ea]] ensure: [stream close]]. (SampledSound fromWaveFileNamed: 'bachFugue.wav') play.
Please review in detail as this is one of my first contacts to the Sound system!
=============== Diff against Sound-nice.69 ===============
Item was changed: ----- Method: AbstractSound>>storeSampleCount:bigEndian:on: (in category 'file i/o') ----- storeSampleCount: samplesToStore bigEndian: bigEndianFlag on: aBinaryStream "Store my samples on the given stream at the current SoundPlayer sampling rate. If bigFlag is true, then each 16-bit sample is stored most-significant byte first (AIFF files), otherwise it is stored least-significant byte first (WAV files). If self isStereo is true, both channels are stored, creating a stereo file. Otherwise, only the left channel is stored, creating a mono file."
| bufSize stereoBuffer reverseBytes streamDirect |
| bufSize stereoBuffer reverseBytes | self reset. bufSize := (2 * self samplingRate rounded) min: samplesToStore. "two second buffer" stereoBuffer := SoundBuffer newStereoSampleCount: bufSize.
streamDirect := aBinaryStream isKindOf: StandardFileStream.reverseBytes := (bigEndianFlag xor: Smalltalk isBigEndian) xor: streamDirect not.
reverseBytes := bigEndianFlag ~= (Smalltalk isBigEndian). 'Storing audio...' displayProgressFrom: 0 to: samplesToStore during: [:bar | | remaining out | remaining := samplesToStore. [remaining > 0] whileTrue: [ bar value: samplesToStore - remaining. stereoBuffer primFill: 0. "clear the buffer" self playSampleCount: (bufSize min: remaining) into: stereoBuffer startingAt: 1. out := self isStereo ifTrue: [stereoBuffer] ifFalse: [stereoBuffer extractLeftChannel]. reverseBytes ifTrue: [out reverseEndianness].
streamDirect
(aBinaryStream isKindOf: StandardFileStream) ifTrue: [ "optimization for files: write sound buffer directly to file" aBinaryStream next: (out size // 2) putAll: out startingAt: 1] "size in words" ifFalse: [ "for non-file streams:" 1 to: out monoSampleCount do: [:i | aBinaryStream int16: (out at: i)]].
remaining := remaining - bufSize]].!
remaining := remaining - bufSize]].- !