shine-mp3 icon indicating copy to clipboard operation
shine-mp3 copied to clipboard

Make safe

Open braheezy opened this issue 2 years ago • 0 comments

There is one last usage of the unsafe package. It would be pretty cool to get rid of it :)

The Unsafe Pointer

In l3subband.go, pointer arithmetic is used in windowFilterSubband:

func (enc *Encoder) windowFilterSubband(buffer **int16, s *[SUBBAND_LIMIT]int32, ch int64, stride int64) {
	y := make([]int32, 64)
	ptr := *buffer

	// replace 32 oldest samples with 32 new samples
	for i := int64(31); i >= 0; i-- {
		enc.subband.X[ch][i+enc.subband.Off[ch]] = int32(*ptr) << 16
		ptr = (*int16)(unsafe.Add(unsafe.Pointer(ptr), unsafe.Sizeof(int16(0))*uintptr(stride)))
	}
	*buffer = ptr
       // Rest of function

The most straightforward thing to do is pass a 2d array instead of a double pointer, and that requires a type change in the Encoder struct and a couple other function edits:

// l3mdct.go
for k := 0; k < 18; k += 2 {
	enc.windowFilterSubband(&enc.buffer[ch], &enc.l3SubbandSamples[ch][gr+1][k], ch, stride)
	enc.windowFilterSubband(&enc.buffer[ch], &enc.l3SubbandSamples[ch][gr+1][k+1], ch, stride)

	// Compensate for inversion in the analysis filter
	// (every odd index of band AND k)
	for band := 1; band < 32; band += 2 {
		enc.l3SubbandSamples[ch][gr+1][k+1][band] *= -1
	}
}

// types.go
type Encoder struct {
	buffer           [MAX_CHANNELS]*int16
}

// layer3.go
func (enc *Encoder) EncodeBufferInterleaved(data []int16) ([]uint8, int) {
	enc.buffer[0] = &data[0]
	if enc.Wave.Channels == 2 {
		enc.buffer[1] = &data[1]
	}
	return enc.encodeBufferInternal(int(enc.Wave.Channels))
}

I've tried the following types for buffer:

  • buffer [MAX_CHANNELS][]int16
  • buffer *[MAX_CHANNELS][]int16

The odd and interesting thing is this simple change drastically changes the quality of the encoded file. I've heard weird things:

  • Good quality, but noticeably lower volume
  • High pitch scratching mixed with music
  • Low garbled noises mixed with music
  • Screeching garbage
  • And everything in between :face_with_spiral_eyes:

braheezy avatar Sep 29 '23 21:09 braheezy