mirror of
https://github.com/librespot-org/librespot.git
synced 2026-04-27 08:15:50 +03:00
[GH-ISSUE #895] ERROR ALSA function 'snd_pcm_hw_params_set_buffer_time_near' failed with error 'EINVAL: Invalid argument' on playback on Vero4K #443
Labels
No labels
A-Alsa
SpotifyAPI
Tokio 1.0
audio
bug
can't reproduce
compilation
dependencies
duplicate
enhancement
good first issue
help wanted
high priority
imported
imported
invalid
new api
pull-request
question
reverse engineering
wiki
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/librespot#443
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @oebeledrijfhout on GitHub (Nov 28, 2021).
Original GitHub issue: https://github.com/librespot-org/librespot/issues/895
Originally assigned to: @roderickvd on GitHub.
On my Vero4K (OSMC) I get the following error on playback:
The librespot command:
I have nothing in
/etc/asound.confor~/.asoundrc. Output ofaplay -l:and
aplay -L:I reported this originally in https://github.com/dtcooper/raspotify/issues/466 there are some more details there.
@JasonLG1979 commented on GitHub (Nov 29, 2021):
To recap The device fails with both
snd_pcm_hw_params_set_buffer_time_nearandsnd_pcm_hw_params_set_buffer_size_nearthere doesn't seem to be a way to set the buffer size on it?@roderickvd it always seems to be these little embedded devices or Kodi these days?
I'm thinking with as much trouble as we have with this it might just be better to move from a hard fail to a best effort and go with:
@JasonLG1979 commented on GitHub (Nov 29, 2021):
@oebeledrijfhout Try this build:
https://drive.google.com/file/d/1IJDvD_iQOYCf0CNZFdQHVAZmrKiUjupp/view?usp=sharing
It's based on this branch:
https://github.com/JasonLG1979/librespot/tree/best-effort-buffer
It doesn't actually fix anything it just makes errors setting the buffer and period size non-fatal.
@oebeledrijfhout commented on GitHub (Nov 29, 2021):
Thank you! I can confirm this allows me to playback, with a
WARNin the log:@roderickvd commented on GitHub (Nov 29, 2021):
I agree that the we should make a change. These all seem to be regressions from the (less optimal) way it was.
@JasonLG1979 commented on GitHub (Nov 29, 2021):
OK, I'll combine the 2 solutions so that
librespotwill make a best effort to size the buffers within the reported min / max reported by alsa but if it fails it will just use the device's default.@JasonLG1979 commented on GitHub (Nov 29, 2021):
Yep, unfortunately just like the warning says not being able to set a reasonable buffer size is not optimal and may result in high CPU usage, high latency, underruns and other audio issues.
In your case in particular your buffer is almost 1.5 secs long with extremely small period sizes. I wouldn't worry about underruns with that many periods in a buffer but with a buffer that large
librespotmay seem "laggy" and with periods that small you're sure to see higher CPU useage.@JasonLG1979 commented on GitHub (Nov 29, 2021):
@oebeledrijfhout if all goes well with the PR that references this issue I will issue a new Raspotify release after it is merged.
@JasonLG1979 commented on GitHub (Nov 29, 2021):
Here's a Raspotify build based on that branch:
https://drive.google.com/file/d/1uXvCve_kzjdrB3upC8rOaDgTLAsBvmdh/view?usp=sharing
See @roderickvd my evil plan of using Raspotify users as
librespottesters is starting to bare fruit 😈 😆.@roderickvd commented on GitHub (Nov 29, 2021):
Good stuff. Would be nice if more would test #896. I'm actively working on #891, but on a Mac, and don't have access to my Alsa rig the coming few days.
@JasonLG1979 commented on GitHub (Nov 29, 2021):
The intent (and so far the case in testing my devices) is that anyone who
librespotworks fine for now will notice nothing. But you know how quirky alsa can be. Worst case though you get the device's default buffer and period values which may not be ideal but IMHO it's better than a hard fail.@oebeledrijfhout commented on GitHub (Nov 29, 2021):
I installed that build and playback is working, with the same warning as with the previous build:
BTW, CPU usage is not looking excessive during playback.
@JasonLG1979 commented on GitHub (Nov 30, 2021):
Yep, that's to be expected.
It's normal for it to spike while it's initially caching the track but after that:
On a Pi Zero less than 20%.
On a Pi 4 around 5%.
I have no idea how the CPU ranks compared to either of those? But I'd imagine it's a lot closer to a Pi 4 than a Zero.
@JasonLG1979 commented on GitHub (Nov 30, 2021):
If it would help to test for anyone else I can build a one off
amd64Raspotify .deb. Just let me know.@JasonLG1979 commented on GitHub (Dec 1, 2021):
@oebeledrijfhout hate to bug you again but I'm still trying to figure out the best way to handle dynamic buffer sizing and would very much appreciate you testing this out:
https://drive.google.com/file/d/1uxqJnj9PhiHgoupKbEspoNpLxOQ_-MTv/view?usp=sharing
@JasonLG1979 commented on GitHub (Dec 1, 2021):
I've set the PR as a draft because with how quirky ALSA is you could test it on 20 different devices and get 20 different results, and I only have a few devices (all of which work with
librespotbefore and after the PR). It needs a lot more testing.@roderickvd commented on GitHub (Dec 1, 2021):
I can test somewhere around the weekend. Maybe @jessicah can test too?
@oebeledrijfhout commented on GitHub (Dec 1, 2021):
No worries, glad to contribute. This build doesn't work on my device:
@roderickvd commented on GitHub (Dec 1, 2021):
You can get it out of draft when you think the code is ready for review. No problem that it needs more testing after that. "Draft" to me means: I'm still at it, but know that this is on the way, and I welcome any input on-the-go.
@JasonLG1979 commented on GitHub (Dec 1, 2021):
What @oebeledrijfhout has shown me is that you can't brute force loop though a range of buffer values and call
set_buffer_size_nearorset_period_size_near.I was hoping to be able to start high and just loop though and stop at an
Ok. So much for that idea. It couldn't be that easy though ofc, it is ALSA after all,lol...@JasonLG1979 commented on GitHub (Dec 2, 2021):
Ok @oebeledrijfhout round, I'm not even sure,lol!!!:
https://drive.google.com/file/d/1NQgwdd7brwe99uMGFfEaYRsUU09yyF9m/view?usp=sharing
This iteration is much like the previous working version except that I've moved the warning about not being able to set the buffer size to a trace message because I figure it's just log noise if someone's not actually having audible issues and if they were hopefully they would file an issue and provide some verbose output. I also added some trace messages to help debug in the case of an issue.
@JasonLG1979 commented on GitHub (Dec 2, 2021):
You should see something like this:
@oebeledrijfhout commented on GitHub (Dec 2, 2021):
looks (and sounds) good!
@JasonLG1979 commented on GitHub (Dec 2, 2021):
Weird that it reports an available buffer size range of
16 - 65536Frames and then totally fails whenlibrespottries to set the buffer to22050Frames which is obviously in that range. That's clearly a driver/configuration bug. The expected result of callingsnd_pcm_hw_params_set_buffer_size_near(and the rust bindings) would be for it to return what the driver set the buffer size to be (hopefully something close to what you asked for). You should basically be able to pass any Frame value to that function and it should never fail. The fact that it always fails is very, very broken. I would file a bug report upstream and tell them to fix their shit drivers/configuration.@JasonLG1979 commented on GitHub (Dec 2, 2021):
I'm glad however that we've figured out a workaround.
@JasonLG1979 commented on GitHub (Dec 2, 2021):
I think they have ghosted us. It must not have been that important to them? But just in case here is an amd64 binary I compiled on my Ubuntu 21.10 box built with
--release --no-default-features --features alsa-backendhttps://drive.google.com/file/d/1oZ45gch7XKTrDi54UemIfvXRenTHQsHb/view?usp=sharing
@sarlej commented on GitHub (Dec 2, 2021):
On my Vero4k (OSMC) version:
raspotify_0.31.3-4-g8d41bd0-dirty~librespot.v0.2.0-150-gc737337_armhf.debfixed my issue (same as @oebeledrijfhout posted originally).I don't have any complains about set buffer size, but i'm using external USB sound card.
@JasonLG1979 commented on GitHub (Dec 2, 2021):
@sarlej that's weird that you had an issue? Judging by your output
librespothas no trouble setting the buffer or period size?It shows that
librespotgot both it's desired buffer and period size (22050and5512) which is as you'd hope it would be?Maybe it's a distro specific issue?
In any event I'm glad if it solves whatever issue you may have been having? And thank you for testing.
@JasonLG1979 commented on GitHub (Dec 2, 2021):
I'd really have to look at their default audio configs to see if they're doing something screwy. They are basically an appliance distro running on what is intended to be an appliance type device, maybe they've changed things to suit there own particular needs at the expense of others?
@sarlej commented on GitHub (Dec 3, 2021):
@JasonLG1979 it looks like it was exactly same issue as @oebeledrijfhout had. I try to reproduce the error and it looks like i didn't configure external USB sound card, so it used internal one. Sorry for mystification.
Nevertheless the update fix the problem when using internal card and log is same as @oebeledrijfhout posted.
@JasonLG1979 commented on GitHub (Dec 3, 2021):
Ok, that makes more sense. Then I guess it's a double confirmation. The fix also worked for you on the affected device and it was not detrimental on a well behaved device (which is also very important) thank you for the clarification.
@JasonLG1979 commented on GitHub (Dec 3, 2021):
@roderickvd I'm going to take the PR out of draft mode since I think the proof of concept is proven. It's really just a matter of further testing and review. I've pushed another commit since I posted those binaries but it didn't change the functionality of the PR. It was mostly just about sprinkling in trace messages to help with future debugging so results from the posted binaries are still applicable.
@roderickvd commented on GitHub (Dec 3, 2021):
Thanks for that work. I'll take some time to test it on my rig this weekend and get it merged. Then when it lives in
devit can get some proper exposure before we release a new version.@JasonLG1979 commented on GitHub (Dec 3, 2021):
In a perfect world of sensible APIs, well behaved devices and logical configurations none of it would be necessary. But as we both know from experience ALSA is not sensible, devices are rarely well behaved and configurations tend to be illogical,lol!!!
@JasonLG1979 commented on GitHub (Dec 3, 2021):
As soon as it hits dev I will ship it in Raspotify. It should be plenty of exposure there.