kerem commented

Owner

Originally created by @pilollipietro on GitHub (Jan 13, 2025).
Original GitHub issue: https://github.com/devgianlu/go-librespot/issues/160

I'm using C.SND_PCM_FORMAT_S16_LE

I attach here code to support it. The patch must be applied to git tag v0.1.3.
go-librespot-SND_PCM_FORMAT_S16_LE.patch.txt

Originally created by @pilollipietro on GitHub (Jan 13, 2025). Original GitHub issue: https://github.com/devgianlu/go-librespot/issues/160 I'm using C.SND_PCM_FORMAT_S16_LE I attach here code to support it. The patch must be applied to git tag v0.1.3. [go-librespot-SND_PCM_FORMAT_S16_LE.patch.txt](https://github.com/user-attachments/files/18400174/go-librespot-SND_PCM_FORMAT_S16_LE.patch.txt)

kerem

2026-02-28 14:25:22 +03:00

closed this issue
added the
enhancement
label

kerem commented

2026-02-28 14:25:23 +03:00

Author

Owner

@devgianlu commented on GitHub (Jan 16, 2025):

You are welcome to make a PR to merge this into the codebase! If you can't/don't want to do it let me know and I'll merge it.

@devgianlu commented on GitHub (Jan 16, 2025): You are welcome to make a PR to merge this into the codebase! If you can't/don't want to do it let me know and I'll merge it.

kerem commented

2026-02-28 14:25:23 +03:00

Author

Owner

@pilollipietro commented on GitHub (Jan 20, 2025):

go-librespot-SND_PCM_FORMAT_S16_LE.patch.txt

Honestly, it's the first patch I've made in go and I don't know rust, I use git for work but I've never used github. I would prefer in the first instance if you can that you did it yourself. Place the patch aligned to the current master

@pilollipietro commented on GitHub (Jan 20, 2025): [go-librespot-SND_PCM_FORMAT_S16_LE.patch.txt](https://github.com/user-attachments/files/18477212/go-librespot-SND_PCM_FORMAT_S16_LE.patch.txt) Honestly, it's the first patch I've made in go and I don't know rust, I use git for work but I've never used github. I would prefer in the first instance if you can that you did it yourself. Place the patch aligned to the current master

kerem commented

2026-02-28 14:25:23 +03:00

Author

Owner

@devgianlu commented on GitHub (Jan 20, 2025):

I have created a PR with your changes, but I had to change the code a little bit and now it does not seem to work. Specifically, errors from configureAdaptiveBuffer were not handled and now it seems to fail there. Can you have a look?

@devgianlu commented on GitHub (Jan 20, 2025): I have created [a PR](https://github.com/devgianlu/go-librespot/pull/166) with your changes, but I had to change the code a little bit and now it does not seem to work. Specifically, errors from `configureAdaptiveBuffer` were not handled and now it seems to fail there. Can you have a look?

kerem commented

2026-02-28 14:25:23 +03:00

Author

Owner

@pilollipietro commented on GitHub (Jan 28, 2025):

Thank you. The problem seems to be that in the else branch the err var is not initializated.
The logic might be
The logic should be that if it fails snd_pcm_hw_params_get_buffer_size it goes to the else branch and prints the error, in reality it might be a warning because in some situations it could work including the one in which you have min fuffer size and max buffer size it could work. I have added some comments in this chunk code I have put some fake comment HEREEEE in the relevant code lines.

If you see my patch I have simply initialize err before the first if. I think that it is all.

func (out *alsaOutput) configureAdaptiveBuffer(hwparams *C.snd_pcm_hw_params_t) error {
	var bufferSize C.snd_pcm_uframes_t
        //HEREEEEEEEEEEEE Initialize Here err was 	err := C.snd_pcm_hw_params_get_buffer_size(hwparams, &bufferSize
       // iferr == 0 {
	if err := C.snd_pcm_hw_params_get_buffer_size(hwparams, &bufferSize); err == 0 {
		// Do the rest only if snd_pcm_hw_params_get_buffer_size returns successfully.
		// If it fails, snd_pcm_hw_params_get_buffer_size will fail and
		// snd_pcm_hw_params_set_buffer_time_near will not work either.
		// In that case, we will use min/max values or adaptive buffers.
		bufferTime := C.uint(out.bufferTime)
		if err := C.snd_pcm_hw_params_set_buffer_time_near(out.pcmHandle, hwparams, &bufferTime, nil); err < 0 {
			return out.alsaError("snd_pcm_hw_params_set_buffer_time_near", err)
		}

		// Request a period size that's approximately bufferSize/4.
		// By default, ALSA might use a very short buffer size (e.g., 220) that could
		// lead to crackling sounds. So we request a buffer with a reasonable period size.
		var periodSize C.snd_pcm_uframes_t = C.snd_pcm_uframes_t(bufferSize) / C.snd_pcm_uframes_t(out.periodCount)
		if err := C.snd_pcm_hw_params_set_period_size_near(out.pcmHandle, hwparams, &periodSize, nil); err < 0 {
			return out.alsaError("snd_pcm_hw_params_set_period_size_near", err)
		}

		return nil
	} else {
		// May be that the configuration space does not contain a single value for buffer size.
		// In this case snd_pcm_hw_params_get_buffer_size_min and snd_pcm_hw_params_get_buffer_size_max are available.
		// snd_pcm_hw_params_set_buffer_time_near will fail but the buffer can be set with snd_pcm_hw_params_set_buffer_time_minmax.
		// snd_pcm_hw_params_set_period_size_near might fail but snd_pcm_hw_params_set_period_size_minmax is available.
		// It may be that in this case nothing needs to be done as the buffers should be adaptive.
                //  HEREEEEEEEEE Print err in first if may be a warning and may be a custom print
                // e.g. warning the driver does not support snd_pcm_hw_params_get_buffer_size 
		return out.alsaError("snd_pcm_hw_params_get_buffer_size", err)
	}
}

@pilollipietro commented on GitHub (Jan 28, 2025): Thank you. The problem seems to be that in the else branch the err var is not initializated. The logic might be The logic should be that if it fails snd_pcm_hw_params_get_buffer_size it goes to the else branch and prints the error, in reality it might be a warning because in some situations it could work including the one in which you have min fuffer size and max buffer size it could work. I have added some comments in this chunk code I have put some fake comment HEREEEE in the relevant code lines. If you see my patch I have simply initialize err before the first if. I think that it is all. ``` func (out *alsaOutput) configureAdaptiveBuffer(hwparams *C.snd_pcm_hw_params_t) error { var bufferSize C.snd_pcm_uframes_t //HEREEEEEEEEEEEE Initialize Here err was err := C.snd_pcm_hw_params_get_buffer_size(hwparams, &bufferSize // iferr == 0 { if err := C.snd_pcm_hw_params_get_buffer_size(hwparams, &bufferSize); err == 0 { // Do the rest only if snd_pcm_hw_params_get_buffer_size returns successfully. // If it fails, snd_pcm_hw_params_get_buffer_size will fail and // snd_pcm_hw_params_set_buffer_time_near will not work either. // In that case, we will use min/max values or adaptive buffers. bufferTime := C.uint(out.bufferTime) if err := C.snd_pcm_hw_params_set_buffer_time_near(out.pcmHandle, hwparams, &bufferTime, nil); err < 0 { return out.alsaError("snd_pcm_hw_params_set_buffer_time_near", err) } // Request a period size that's approximately bufferSize/4. // By default, ALSA might use a very short buffer size (e.g., 220) that could // lead to crackling sounds. So we request a buffer with a reasonable period size. var periodSize C.snd_pcm_uframes_t = C.snd_pcm_uframes_t(bufferSize) / C.snd_pcm_uframes_t(out.periodCount) if err := C.snd_pcm_hw_params_set_period_size_near(out.pcmHandle, hwparams, &periodSize, nil); err < 0 { return out.alsaError("snd_pcm_hw_params_set_period_size_near", err) } return nil } else { // May be that the configuration space does not contain a single value for buffer size. // In this case snd_pcm_hw_params_get_buffer_size_min and snd_pcm_hw_params_get_buffer_size_max are available. // snd_pcm_hw_params_set_buffer_time_near will fail but the buffer can be set with snd_pcm_hw_params_set_buffer_time_minmax. // snd_pcm_hw_params_set_period_size_near might fail but snd_pcm_hw_params_set_period_size_minmax is available. // It may be that in this case nothing needs to be done as the buffers should be adaptive. // HEREEEEEEEEE Print err in first if may be a warning and may be a custom print // e.g. warning the driver does not support snd_pcm_hw_params_get_buffer_size return out.alsaError("snd_pcm_hw_params_get_buffer_size", err) } } ```

kerem commented

2026-02-28 14:25:23 +03:00

Author

Owner

@devgianlu commented on GitHub (Jan 28, 2025):

I still don't get it because snd_pcm_hw_params_get_buffer_size returns an error on my system, but the snd_pcm_hw_params_set_buffer_time_near and snd_pcm_hw_params_set_period_size_near calls work anyway. Contrary to what you mention in the comment:

If it fails, snd_pcm_hw_params_get_buffer_size will fail and snd_pcm_hw_params_set_buffer_time_near will not work either.

@devgianlu commented on GitHub (Jan 28, 2025): I still don't get it because `snd_pcm_hw_params_get_buffer_size` returns an error on my system, but the `snd_pcm_hw_params_set_buffer_time_near` and `snd_pcm_hw_params_set_period_size_near` calls work anyway. Contrary to what you mention in the comment: > If it fails, snd_pcm_hw_params_get_buffer_size will fail and snd_pcm_hw_params_set_buffer_time_near will not work either.

kerem commented

2026-02-28 14:25:23 +03:00

Author

Owner

@pilollipietro commented on GitHub (Jan 29, 2025):

ALSA Buffer Size Analysis

Ok may be that it is, my assumption is wrong.

FACTS (or believe?):

There are two kinds of sound card class devices:

One with only a single buffer size.
Another with a minimum and maximum buffer size.

If snd_pcm_hw_params_get_buffer_size fails, the algorithm to set parameters does not work correctly because the computation depends on bufferSize:

var periodSize C.snd_pcm_uframes_t = C.snd_pcm_uframes_t(bufferSize) / C.snd_pcm_uframes_t(out.periodCount)

The if branch also fails if snd_pcm_hw_params_set_buffer_time_near or snd_pcm_hw_params_set_period_size_near fail.

Testing on Your System

Run the following command:

aplay -D iec958:CARD=AMLM8AUDIO,DEV=0 --dump-hw-params /dev/zero

Replace -D with your device found using:

aplay -L

If you see BUFFER_SIZE: [48 16384], then snd_pcm_hw_params_get_buffer_size and snd_pcm_hw_params_set_buffer_size might work.
If you see BUFFER_MIN_SIZE and BUFFER_MAX_SIZE, then the if branch could not work, as you cannot use snd_pcm_hw_params_get_buffer_size and snd_pcm_hw_params_set_buffer_size.
(Reference)
May be that not all drivers follow the same behavior. Some allow setting a specific buffer size within the min-max range, while others auto-adapt. You might want to mention that this depends on the ALSA implementation for the specific hardware.

Assumption

If the card supports snd_pcm_hw_params_get_buffer_size_min and snd_pcm_hw_params_get_buffer_size_max, it might be adaptive (ALSA does not explicitly document this behavior or I have not found it). In this case, setting these parameters is not useful, as the buffer size changes automatically.

Proposal

We could wrap the old (if branch) code in a try-catch equivalent and log all errors as warnings.

Curiosity: Did you write this code? Does this conversation seem familiar to you?

@pilollipietro commented on GitHub (Jan 29, 2025): # ALSA Buffer Size Analysis Ok may be that it is, my assumption is wrong. ## FACTS (or believe?): There are two kinds of sound card class devices: 1. One with only **a single buffer size**. 2. Another with **a minimum and maximum buffer size**. If `snd_pcm_hw_params_get_buffer_size` fails, the algorithm to set parameters does not work correctly because the computation depends on `bufferSize`: ```c var periodSize C.snd_pcm_uframes_t = C.snd_pcm_uframes_t(bufferSize) / C.snd_pcm_uframes_t(out.periodCount) ``` The `if` branch also fails if `snd_pcm_hw_params_set_buffer_time_near` or `snd_pcm_hw_params_set_period_size_near` fail. ## Testing on Your System Run the following command: ```sh aplay -D iec958:CARD=AMLM8AUDIO,DEV=0 --dump-hw-params /dev/zero ``` Replace `-D` with your device found using: ```sh aplay -L ``` - If you see **`BUFFER_SIZE: [48 16384]`**, then `snd_pcm_hw_params_get_buffer_size` and `snd_pcm_hw_params_set_buffer_size` **might work**. - If you see **`BUFFER_MIN_SIZE` and `BUFFER_MAX_SIZE`**, then the `if` branch **could not work**, as you **cannot use** `snd_pcm_hw_params_get_buffer_size` and `snd_pcm_hw_params_set_buffer_size`. ([Reference](https://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m___h_w___params.html#gab6556fcaaf926360d2064044a6f6cfb4)) May be that not all drivers follow the same behavior. Some allow setting a specific buffer size within the min-max range, while others auto-adapt. You might want to mention that this depends on the ALSA implementation for the specific hardware. ## Assumption If the card supports `snd_pcm_hw_params_get_buffer_size_min` and `snd_pcm_hw_params_get_buffer_size_max`, it might be adaptive (ALSA does not explicitly document this behavior or I have not found it). In this case, setting these parameters is not useful, as the buffer size changes automatically. ## Proposal We could wrap the old (if branch) code in a `try-catch` equivalent and log all errors as warnings. --- **Curiosity:** Did you write this code? Does this conversation seem familiar to you?

kerem commented

2026-02-28 14:25:23 +03:00

Author

Owner

@pilollipietro commented on GitHub (Jan 31, 2025):

Read the FFmpeg Alsa code it has a similar logic, and I'm pretty sure that it works on all systems.

00122     snd_pcm_hw_params_get_buffer_size_max(hw_params, &buffer_size);
00123     /* TODO: maybe use ctx->max_picture_buffer somehow */
00124     res = snd_pcm_hw_params_set_buffer_size_near(h, hw_params, &buffer_size);
00125     if (res < 0) {
00126         av_log(ctx, AV_LOG_ERROR, "cannot set ALSA buffer size (%s)\n",
00127                snd_strerror(res));
00128         goto fail;
00129     }
00130 
00131     snd_pcm_hw_params_get_period_size_min(hw_params, &period_size, NULL);
00132     if (!period_size)
00133         period_size = buffer_size / 4;
00134     res = snd_pcm_hw_params_set_period_size_near(h, hw_params, &period_size, NULL);
00135     if (res < 0) {
00136         av_log(ctx, AV_LOG_ERROR, "cannot set ALSA period size (%s)\n",
00137                snd_strerror(res));
00138         goto fail;
00139     }
00140     s->period_size = period_size;

Reading this code it seems that they don't set buffer time and they set buffer size to max. May be a reasonable choice.

@pilollipietro commented on GitHub (Jan 31, 2025): Read [the FFmpeg Alsa code](https://ffmpeg.org/doxygen/0.6/alsa-audio-common_8c-source.html.) it has a similar logic, and I'm pretty sure that it works on all systems. ``` 00122 snd_pcm_hw_params_get_buffer_size_max(hw_params, &buffer_size); 00123 /* TODO: maybe use ctx->max_picture_buffer somehow */ 00124 res = snd_pcm_hw_params_set_buffer_size_near(h, hw_params, &buffer_size); 00125 if (res < 0) { 00126 av_log(ctx, AV_LOG_ERROR, "cannot set ALSA buffer size (%s)\n", 00127 snd_strerror(res)); 00128 goto fail; 00129 } 00130 00131 snd_pcm_hw_params_get_period_size_min(hw_params, &period_size, NULL); 00132 if (!period_size) 00133 period_size = buffer_size / 4; 00134 res = snd_pcm_hw_params_set_period_size_near(h, hw_params, &period_size, NULL); 00135 if (res < 0) { 00136 av_log(ctx, AV_LOG_ERROR, "cannot set ALSA period size (%s)\n", 00137 snd_strerror(res)); 00138 goto fail; 00139 } 00140 s->period_size = period_size; ``` Reading this code it seems that they don't set buffer time and they set buffer size to max. May be a reasonable choice.

kerem commented

2026-02-28 14:25:23 +03:00

Author

Owner

@pilollipietro commented on GitHub (Jan 31, 2025):

This code works perfectly on my two laptops and on the ARM TV box connected to the DAC of the HiFi system. It is similar to the original code, with key differences: it does not read the bufferTime variable and sets the largest possible buffer. If I uncomment the setBufferTime, setting it to a valid parameter causes the ALSA driver to crash on initialization with a generic hwParams error. I think the problem lies in the consistency between bufferTime and bufferSize; in any case, I believe this is the best option.

func (out *alsaOutput) configureAdaptiveBuffer(hwparams *C.snd_pcm_hw_params_t) error {
        var bufferSize C.snd_pcm_uframes_t
        if err := C.snd_pcm_hw_params_get_buffer_size_max(hwparams, &bufferSize); err < 0 {
                return out.alsaError("snd_pcm_hw_params_set_buffer_size_max", err)
        }
        if err := C.snd_pcm_hw_params_set_buffer_size_near(out.pcmHandle, hwparams, &bufferSize); err < 0 {
                return out.alsaError("snd_pcm_hw_params_set_buffer_size_near", err)
        }
        //bufferTime := C.uint(out.bufferTime)
        //if err := C.snd_pcm_hw_params_set_buffer_time_near(out.pcmHandle, hwparams, &bufferTime, nil); err < 0 {
        //      return out.alsaError("snd_pcm_hw_params_set_buffer_time_near", err)
        //}
        var periodSize C.snd_pcm_uframes_t = C.snd_pcm_uframes_t(bufferSize) / C.snd_pcm_uframes_t(out.periodCount)
        if err := C.snd_pcm_hw_params_set_period_size_near(out.pcmHandle, hwparams, &periodSize, nil); err < 0 {
                return out.alsaError("snd_pcm_hw_params_set_period_size_near", err)
        }
        return nil

@pilollipietro commented on GitHub (Jan 31, 2025): This code works perfectly on my two laptops and on the ARM TV box connected to the DAC of the HiFi system. It is similar to the original code, with key differences: it does not read the bufferTime variable and sets the largest possible buffer. If I uncomment the setBufferTime, setting it to a valid parameter causes the ALSA driver to crash on initialization with a generic hwParams error. I think the problem lies in the consistency between bufferTime and bufferSize; in any case, I believe this is the best option. ``` func (out *alsaOutput) configureAdaptiveBuffer(hwparams *C.snd_pcm_hw_params_t) error { var bufferSize C.snd_pcm_uframes_t if err := C.snd_pcm_hw_params_get_buffer_size_max(hwparams, &bufferSize); err < 0 { return out.alsaError("snd_pcm_hw_params_set_buffer_size_max", err) } if err := C.snd_pcm_hw_params_set_buffer_size_near(out.pcmHandle, hwparams, &bufferSize); err < 0 { return out.alsaError("snd_pcm_hw_params_set_buffer_size_near", err) } //bufferTime := C.uint(out.bufferTime) //if err := C.snd_pcm_hw_params_set_buffer_time_near(out.pcmHandle, hwparams, &bufferTime, nil); err < 0 { // return out.alsaError("snd_pcm_hw_params_set_buffer_time_near", err) //} var periodSize C.snd_pcm_uframes_t = C.snd_pcm_uframes_t(bufferSize) / C.snd_pcm_uframes_t(out.periodCount) if err := C.snd_pcm_hw_params_set_period_size_near(out.pcmHandle, hwparams, &periodSize, nil); err < 0 { return out.alsaError("snd_pcm_hw_params_set_period_size_near", err) } return nil ```

kerem commented

2026-02-28 14:25:23 +03:00

Author

Owner

@devgianlu commented on GitHub (Sep 3, 2025):

It took a while, but I merged support for more PCM formats. I did not include the adaptive buffer part, please open a separate issue if that's still an issue for you.

@devgianlu commented on GitHub (Sep 3, 2025): It took a while, but I merged support for more PCM formats. I did not include the adaptive buffer part, please open a separate issue if that's still an issue for you.

kerem commented

2026-02-28 14:25:24 +03:00

Author

Owner

@Gerrelt commented on GitHub (Sep 26, 2025):

Thank you for the merge! The new version 0.4.0 solved my problem as mentioned in #200 .

@Gerrelt commented on GitHub (Sep 26, 2025): Thank you for the merge! The new version 0.4.0 solved my problem as mentioned in #200 .

kerem referenced this issue

2026-02-28 14:26:15 +03:00

[PR #103] [MERGED] Use configuration directory and store app state there #209

Rows
Columns

[GH-ISSUE #160] go-librespot works with driver that support C.SND_PCM_FORMAT_FLOAT_LE only #100

ALSA Buffer Size Analysis

FACTS (or believe?):

Testing on Your System

Assumption

Proposal