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, [hidden email] 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]].
> - !
>
>

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

> <http://www.hpi.de/>
>
> Best,
> Christoph
> ________________________________
> Von: Squeak-dev <[hidden email]> im Auftrag von David T. Lewis <[hidden email]>
> Gesendet: Dienstag, 1. September 2020 04:13:16
> An: [hidden email]
> Cc: [hidden email]
> 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, [hidden email] 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]].
> > - !
> >
> >
>

>