Certain files will not play with ogg123, e.g.:

~$ ogg123 /data/Sounds/Barbara_Allen.ogg

Audio Device:   Advanced Linux Sound Architecture (ALSA) output

Playing: /data/Sounds/Barbara_Allen.ogg
Ogg Vorbis stream: 1 channel, 44100 Hz
ALSA snd_pcm_hw_params_set_channels error: Invalid argument
Error: Cannot open device alsa.

Others play fine.
Fails:
http://upload.wikimedia.org/wikipedia/commons/a/ad/Barbara_Allen.ogg
Plays OK:
http://upload.wikimedia.org/wikipedia/commons/2/2c/FootprintsInTheSnow.ogg

This may be related to mono files, as discussed at:
http://www.mail-archive.com/gnewsense-dev@nongnu.org/msg02135.html

Hi John,

this bug (vorbis-tools: Sends invalid argument to ALSA when playing some files)
seems to have been a bug with an old libao.

The problematic file works fine for me on unstable.

Can you confirm on an unstable, stable or oldstable system that the
problem is gone?

Thanks
Adrian

Hi John,

this bug (vorbis-tools: Sends invalid argument to ALSA when playing some files)
seems to have been a bug with an old libao.

The problematic file works fine for me on unstable.

Can you confirm on an unstable, stable or oldstable system that the
problem is gone?

Thanks
Adrian

tags 578720 wontfix
thanks


Hi John!
output device doesn't support Mono playback and no software in the
output pipeline (that probably looks somewhat like the following in your
case) is willing to do the necessary upmixing:

ogg123 -> libao -> ALSA -> device driver -> hardware

Doing the upmixing in ogg123 is certainly out of its scope and libao
also seems too simple. ALSA has plugins to support upmixing [1] [2]. The
device driver or the hardware are certainly harder to replace. You could
also put a sound server like pulseaudio in the middle, that could do the
upmixing, too.

Alsacap [3] seems like an interesting tool to check, which modes your
current ALSA configuration supports.

Cheers,
Martin


[1]
https://wiki.archlinux.org/index.php/Advanced_Linux_Sound_Architecture#Upmixing
[2] https://packages.debian.org/jessie/libasound2-plugins
[3] http://www.volkerschatz.com/noise/alsa.html#alsacap

severity 578720 minor
thanks


Martin Steghöfer wrote:

Hmmm, however, what could be done about this issue is to improve the
error message, so it tells the user that the mode isn't supported by the
user's hardware, instead of talking about an "invalid argument".
Unfortunately, this message isn't produced by ogg123. The string
"invalid argument" comes from ALSA ("snd_strerror") and it's printed by
libao (in "ao_plugin_open"). So either of them could improve the wording.

What do the libao maintainers (Ron and John in CC) think about this?
Should I reassign this to the libao package?

Cheers,
Martin

Hi Martin,

Actually, it's not clear to me that this error message is coming out of
libao at all.  Or at least nothing I see in the libao source corresponds
to the interesting part of the original report:

 Playing: /data/Sounds/Barbara_Allen.ogg
 Ogg Vorbis stream: 1 channel, 44100 Hz
 ALSA snd_pcm_hw_params_set_channels error: Invalid argument
 Error: Cannot open device alsa.


The last line there comes out of ogg123 after ao_open_live() has failed
(which failed because the snd_pcm_hw_params_set_channels() alsa-lib call
failed to configure alsa to use 1 channel on the user's hardware).
And I assume the reason it worked for Adrian is that his audio hardware
does support that.

The second to last line appears to have come from alsa-lib itself,
I don't see anything in libao that would emit that.  And it combined
with the previous line (1 channel, 44100 Hz) should at least hint at
the idea that "set_channels(1)" failed.

It might be a little counter-intuitive until you realise some hardware
simply can't handle mono output, but it's not clear to me that anything
above the level of alsa-lib can really second-guess (or report) why it
failed in any more detailed way.


It's not really a bug in libao as such; ogg123 tells libao the format of
the audio it wants to stream to it, and libao can either open a stream
that can accept that format or it can't.  It's not really libao's job to
try to guess how it might resample or remix the input to a format that
is acceptable.

Whether or not it's a bug in ogg123 really depends on whether you think
that application should be able to resample and/or remix input that the
hardware does not directly support.  I'm pretty sure things like aplay
will also fail in the same way for the same combination of input and
hardware (at least I've seen it do the same thing for mono files on
some hardware before too).

The choices are basically:

 - Use a "higher level" player that can resample/remix.
 - Manually do that before using simple tools like ogg123/aplay etc.
   to play it.
 - Make ogg123 a "higher level" player.
 - Use an output stream other than alsa which can do this.
 - Use better hardware.


You'd see exactly the same sort of failure if the input stream was in
some other form that wasn't supported by the hardware/alsa driver too
(ie. multiple channel surround or an oddball sample rate).


So sorry, I don't really see an easy out that just lets you punt this
one to someone else :)  But it's also not really a "bug" in ogg123
either, except possibly if you want to document this in more detail
for it (if it's not already and I just can't see it).  It could in
theory try to use a bigger hammer to force a square peg into a round
hole, but that's probably out of scope for a very simple player like
this one.

  Cheers,
  Ron

Hi Ron!

Thank you for your quick and comprehensive answer! :-)

Unfortunately, I think I should have pasted more information into my
previous mail. Most of your answer tries to argue away a point that I
actually already ruled out in a message I posted earlier to this bug
report: It looks like we both agree that doing the upmixing is out of
the scope of both ogg123 and libao. I didn't suggest to do that. I
thought the snippet I pasted into my message to you made that clear. But
apparently it didn't, I'm sorry for not having made that more clear and
therefore having wasted your time. I have a very compact way of writing
things, but I try to be more verbose this time.

After upmixing is ruled out, the only thing left is to (maybe) improve
the error message, if that's possible.

In ogg123 we don't have the information that would be necessary in order
to tell what's going on. The only thing ogg123 knows is that for some
reason the ALSA output couldn't be opened. And that's what ogg123
reports to the user ("Error: Cannot open device alsa."). I don't see any
way to improve this.

So, if anything, we can only improve the *other* message. And that one
"comes" from ALSA (the error code and its translation into a string) and
is "emitted" (converted to string, composed and printed) by libao, like
I tried to explain in my previous message:

Since you were doubting this...

...let me be more specific: The line in which libao emits this is
located in ao_alsa09.c:391 in version 0.8.8 (which is the one the bug
reporter used). In the most recent version (1.1.0) a similar output
would be emitted by ao_alsa.c:521.

Exactly, *that* is the (only) point where I see potential for
improvement. The function name "snd_pcm_hw_params_set_channels" probably
doesn't mean much to the user. And "invalid parameters" sounds more like
an internal software problem (one of those illegal function parameters
that you try to detect with assertions) than like a lack of hardware
features.

Not clear to me, either. That's why I'm coming to you because you know
libao better. An idea from my side: libao *can* know what's going on by
combining the following information: The fact that the error happened,
when setting the output device parameters, and the return code (an
integer value that means "invalid parameters"). The information in
"internal->cmd" could even tell *which* parameter couldn't be set (in
this case the number of channels).

I'm not saying that anything *has* to be done, neither am I saying that
libao is the right place to do say. I'm just saying that *if* we want to
do anything to improve this, the only places that occur to me for doing
this in a reasonable way are libao or ALSA. That's why I came to you, to
hear your opinion, if you think this should be done in libao, in ALSA or
not at all.

I wasn't trying to. This is not about *who* does things (I'd be glad to
contribute, be the "issue" in vorbis-tools or in libao; reassigning
doesn't mean that I'm out), but *where* things are done. Since it was
clear to me that the issue couldn't be helped in vorbis-tools, instead
of just closing the issue I tried a final shot: Searching an external
place, where the issue might be mitigated. And I still see a slight
chance for this to happen in libao.

Thank you for your help! :-)

Cheers,
Martin

I did actually go read the whole bug report (since there indeed wasn't
quite enough quoted to get the whole picture of what this was about :)

No, that's fine.  Most of that was just me "thinking out loud" as I traced
through the relevant code.  That we agree on that just means we came to the
same conclusion.  I didn't mean to suggest you thought otherwise, I was
just fleshing out the reasons I thought that was the right call to make.

Sorry if that wasn't clear (or too verbose to immediately make sense :)

I don't think we have enough in libao to do this either.  It would
need to be done in alsa-lib ...

Ah, right.  That was the bit was missing, that this was against a much
older version of libao, and the code emitting that had changed quite a
bit since then (including the message that would be output).  I'd only
looked at the current code.

That doesn't substantially change what I was saying though.  In libao
(that we currently have), the code in question is this:

	err = snd_pcm_hw_params_set_channels(internal->pcm_handle,
			params, (unsigned int)device->output_channels);
	if (err < 0){
          adebug("snd_pcm_hw_params_set_channels() failed.\n");
          return err;
        }

And all we really know is that call into alsa-lib failed.  We don't
really know anything more about *why* it failed than the error code
it returns.  If all it returns is "Invalid argument", that's all we
have to go on.

We can't know that was because the hardware didn't support the
requested number of channels (if alsa doesn't explicitly say that)
it could be for any number of reasons.

All we really know is the snd_pcm_hw_params_set_channels call failed,
and that's what we report.

Right, but "Set hardware channels failed" doesn't explain it much
differently.  And "invalid parameters" is all alsa told us.

I'm not specifically objecting to improving that, but to do so
it would have to be in alsa-lib, not libao.  And then there'd
be some risk of breaking something else using alsa if it makes
assumptions about the error codes that might be returned.

The other option would be for ogg123 to try to query the output
capabilities before deciding whether it can legally specify them,
but it's not clear that added complexity is worth it either ...

No, we still don't know *why* it failed.  All we know is which alsa-lib
function we called, and the error code it returned.  Which is what we
already report.

I don't think there's much we can do in libao, at least not without
making things a lot more complicated by querying capabilities rather
than just passing on the error if we aren't able to set what was
requested, and arguably that's a job for the app using libao to do
before it requests those capabilities from libao anyway (and even
then in neither case are you guaranteed that the call into alsa-lib
still won't fail for some other reason).

You might be able to do something more in ALSA, but whether that's
a good idea, and whether its maintainers would be keen to do it
is a question you'd have to run past them.

That wasn't meant to sound like an accusation :)  More like sympathy that
I don't really see a lot of practical ways to deal with this (aside from
what you'd already noted in the bug report).

We're basically talking about very simple low level tools, if you ask
them to do something they can't do, you get very simple low level errors
in response.  It's not a bad thing to try to improve that, but it becomes
a game of whack a mole, where by the time you've done everything you need
to produce a more detailed explanation of what they mean for a novice user,
you might as well add the extra code to actually fix that too ...

... and then before you know it, you no longer have a simple low level
tool, but a high level all-singing, all-dancing, audio player.

So it's mostly a question of where we draw the line, since players that
do that do already exist too.

Thank you for trying to bring some closure to this 4 year old bug!

You could run it past the ALSA maintainers and see if they think they
could do something better here.  But if not, I'd probably just close
it now (or close it with a patch sent upstream to note this in the
ogg123 docs, which might be useful regardless of what the ALSA folk do).

It's not really "wontfix", and not even really a problem that will
affect all users.  If the original reporter had audio hardware that
did support mono streams (as some do), or if they'd been using a
different(ly configured) backend, they wouldn't have hit this with
that file either.

There are lots of potential 'solutions' to this, at lots of layers
in the audio stack, and what the right answer for individual users
will be is probably almost as varied.  Which is why all I can really
do is muse about options, there isn't an obvious trivial patch for
either ogg123 or libao that we can just apply to Make It All Better.

  Best,
  Ron

Hi Ron!

Ron wrote:

Ah, OK, that's good :-)

True, but that may be all we need, if we interpret it in the context.
The wording is bad because ALSA tries to use generic error codes
wherever possible, in this case the EINVAL ("Invalid argument") error
code from the linux system errors. It also delegates the
int-to-string-translation to the linux system errors function "strerror"
- which, of course, cannot know the ALSA context.

Is there any other reason for this function to fail with EINVAL other
than the device not supporting the mode we've asked for? Which other
"invalid argument" could there be? The other arguments we pass to the
function have to be OK at that point.

A very quick look at the ALSA code seems to confirm that EINVAL is
returned if and only if after ruling out all configurations that don't
comply with the number of channels we've asked for, the remaining set of
configurations would be empty.

To me, that seems to be enough to interpret EINVAL in *this* *concrete*
place as "device doesn't support the output mode we've asked for".

It could fail for a number of other reasons, but in those other cases it
wouldn't report EINVAL, would it?

The string you suggested would certainly more readable. But that part
isn't the main point, I think the other part is more important:

Yes, but this part may be interpreted in the context to provide a more
detailed error message (see my explanation above).

If we can't work anything out for libao, then that's the next (and last)
thing I would try. But I see more chances for this to happen in libao.
If ALSA sticks to generic Linux system error codes almost everywhere,
then they probably won't abandon that principle for this little issue.
And then there's the risk of breaking other software by changing the
error code, like you said. :-(

That would at least keep the users from thinking that "ogg123" or
"libao" is broken - at least those that read documentation ;-)

Thanks again for you collaboration!

Cheers,
Martin

The problem is, that even if this is the case with the current alsa
code, we have no guarantee it would always remain true.  And it may
in turn pass back error codes from the kernel, which could include
this one too.

(which is why I didn't bother looking at the alsa-lib code to see
if there was a way we could do something 'clever' here.  Its next
release could look nothing like the code in the current one in its
internals, even if it's otherwise API/ABI compatible)

Without some firm API guarantee from alsa-lib, that this is *exactly*
what this means, and it will never mean something else, this is at best
"reading tea-leaves".  At worst it means we'll misinterpret some future
error code return and return some message that's not just a bit tricky
to interpret, but is actively misleading and wrong.

And there's still a long list of things other than number of channels
which this could fail on in much the same way (sample rate, sample
format, sample size, etc.)

I think anything which wants to verify those things in a user friendly
manner really needs to check them for validity specifically itself,
rather than just throwing them at libao to see if they'll stick, and
hoping it will explain why not in a way suitable for them.

In the current libao code, I don't think this will even output a
message at all unless you've turned the verbosity up (which is
different to the code that this was originally seen on).

Right, including breaking libao if we were to do the trick you proposed
above :)

But yeah, unless alsa-lib also has a verbose debug mode, I also doubt
they'll be wanting to change the normal error returns.  And libao in
turn just passes those along verbatim back to the original caller.

If you have the function that was called, and the error code it returned
that's at least something concretely meaningful that you can search for.
If we hide that under some 'magic' interpretation, and the magic goes
wrong, we'll send you on a wild goose chase unless you first read the
libao code to decode the magic.

Well, the ones who don't read documentation (or source :) are probably
still going to think/report that ogg123 is broken even if it tells them
"Sorry I can't play 1 channel audio on your system!", so I'm not sure
we can really ever win that game.

Documenting that it is a known limitation for some hardware/backends
probably really is about the best we can reasonably do here without
making ogg123 a much more complicated application.

  Cheers,
  Ron

Hi Ron!

I agree that implementing some "magic" to find out the real reason isn't
a very good idea. I think the main point where we still disagree is how
"magic" (according to you) or "specified" (according to me) the meaning
of EINVAL really is in this situation.

Ron write:

Technically it *can* do a lot of things, but it isn't allowed to.
Passing on an "EINVAL" error code from somewhere internal is something I
would consider a bug. If a callee returns an "invalid arguments" code to
the caller, that information has to refer to the actual call at hand,
instead of some call somewhere under the hood. In the latter case a
return value "EINVAL" would be very misleading. (I admit, however, that
some programmers might feel tempted to just pass on error codes
unchecked, especially because in C you cannot have container exceptions
wrapped around the original exception like in OO languages.)

If a function "set number of channels" reports an invalid argument, to
me the only correct interpretation of this is that the number of
channels that I've asked for isn't allowed.

And no, the ALSA documentation itself doesn't say that explicitly. But
other (more general) references for EINVAL do so. And I consider that
common sense.

But I can understand that to you don't want to trust too much in other
programmers' care for reasonable error codes and rather stick with
whatever error string they hand you. (Although I'm afraid that with that
level of trust you probably have very little to base on in general,
seeing how few things are actually specified in an exact and/or formal
way - unfortunately!).

You are right that whatever is under the hood should be irrelevant to
us. I shouldn't have mentioned that I looked at the implementation. That
was more of a side note than an argument. I'm just discussing the
meaning of the API (even though I know what is under the hood).

They could fail with the same error code, but not in the same function
call. If the callee tells us that the argument was invalid, that should
(and does in this case) refer to the actual argument at hand.

That's a different path to solve the same issue. I didn't know that the
libao API allowed us to do so. In any case, this would probably be an
excessive effort for this issue.

But in general, independently from the quality of ALSA/libao's error
message, implementing this would be a good thing. Can you point me in
the right direction? At first sight I haven't found a libao function to
check that.

Or were you thinking about checking device capabilities using ALSA
directly? That would somehow foil the point of using libao as
intermediary in the first place.

I'm going to check that suspicion. If it is correct, that would render
our hole discussion pointless. :-(

No, our check for the EINVAL return code would just never become "true"
and therefore we would be back to using the strings handed to us by
"snd_strerror". Our "trick" (as you call it) wouldn't be outside the
specification. But the ALSA return code change could break applications
that consider EINVAL the *only* valid error code in the case (which they
shouldn't).

We should wrap this up before we get lost in a too general discussion:
While I still think that "mode not supported by ALSA device" is the only
valid conclusion that can be drawn from receiving an "invalid parameter"
error on that concrete function call, I respect your concern about ALSA
programmers maybe not following that logic and just passing on internal
error codes (be it due to mistake, program performance, laziness or
something else). Changing the error message is not my decision to make,
I was only suggesting a possible improvement.

So I will check, if I report this to ALSA (probably a pointless
exercise) and if some documentation improvements can still be made in
ogg123 (not sure) and then close this "bug" (which it probably never
really was).

Thank you for having made the effort of discussing this minor issue in
such detail!

Cheers,
Martin

No, what I mean is we pass more than one parameter in these function
calls.  It would be perfectly fine for alsa-lib to sanity check all
of them and return EINVAL if any of them were invalid, for whatever
reasons.

It would likewise be fine for it to pass them further up the chain
to the kernel (which it should do in the final snd_pcm_hw_params()
call, if not also before), and for *it* to return EINVAL if some
part of that is invalid (and for alsa-lib to then return that to
us again).

I'm not saying any of these things are currently the case, but they
seem like perfectly reasonable things that any future implementation
might begin to do (or past implementation used to do) without having
specific API guarantees to the contrary.

Then what would it return if one of the other arguments passed to that
function were invalid?

It's fine to look.  That's why having the source is important :)
That's just different from what we can safely assume will remain
true into the future.

I just didn't look, because I don't think we can safely assume
anything except "it may fail for any number of reasons, and if
it does we'll get an error code back".  I think trying to layer
extra meaning on that purely by making assumptions of our own
would be a somewhat fragile thing to do.

Right, but now we need to make magic assumptions about the return codes
of *many* functions.  Otherwise the first person to try to play a file
with a 12345Hz sample rate, that their hardware doesn't support, will
be right back where we started with this report.

I don't think libao actually does have an interface for that :)
But alsa does.  I think it was one of the complaints of pulse that it
makes determining this sort of thing nearly (or totally) impossible
too - and since libao is a generic front end, it gets a bit tricky
too (though you can at least query which backend libao is using).

It is the right way to do this if you want to be able to give detailed
user friendly reporting of what is valid or not though, or if you want
to ensure this really can play any file regardless of what the hardware
or its drivers support.

But yes, it seems like overkill for ogg123.  If you want a more full
featured player there are plenty of those to choose from.

libao itself is also just meant to be a very simple abstraction.
I think Monty has been hoping for years that someone would write
something to replace it that doesn't suck so we could kill it off.

We're, uh ...  still waiting.  Unfortunately.

He might take patches to add this, but it could be tricky to do for
all of the backends in a generic way.

 #define adebug(format, args...) {\
    if(device->verbose==2){                                             \
      if(strcmp(format,"\n")){                                          \
        if(device->funcs->driver_info()->short_name){                   \
          fprintf(stderr,"ao_%s debug: " format,device->funcs->driver_info()->short_name,## args); \
        }else{                                                          \
          fprintf(stderr,"debug: " format,## args);                     \
        }                                                               \
      }else{                                                            \
        fprintf(stderr,"\n");                                           \
      }                                                                 \
    }                                                                   \
  }

Which would then leave us with an awful mess of having to do different
things based on different versions of alsa etc.  Making it even more
difficult for someone trying to work backward through the magic to
find out what really failed.

This "probably won't happen", but it's the kind of risk we expose
ourselves to if we try to be "clever" here.  And it's still only
speculation that rewording the error message a bit will actually
really help anyone.


At the end of the day, it's still not going to play files that aren't
supported by the hardware when using the direct ALSA backend.  Which
is what most people will probably consider "the bug" to be if they
encounter it.

The original reporter of this bug already concluded that the mono
file was most probably their problem.  So the existing message was
sufficient for that it would seem.

What they apparently didn't know is that this was "expected" in the
configuration they were using, and not just some oversight.  I don't
think we can fix that by changing the error message as such.

"Error: successfully failed to play 1 channel file on your hardware"
isn't really much of a consolation if you were expecting that to work :)

Except even that is misleading if installing the remixing plugin, or using
some other front end to alsa as the backend to libao would still let this
work (regardless of the case where it failed due to the other parameters
we passed to the function).

Which is why talking through this is basically pushing me toward "note this
stuff in the documentation" as about the only viable solution.

Linux audio is still a mess in many ways.  Of the dozens of ways any given
user can set it up to work for themselves, each has their own set of pros
and cons still, and will vary from user to user depending on what hardware
they have.

I think we've pretty much already arrived at what the right answer for
ALSA will be (don't change anything or you'll break stuff!), so yeah,
I think being a bit more explicit in the ogg123 docs is probably the
best bet at this stage.

That nobody has complained about this a lot since 2010, might mean they
aren't using the raw ALSA backend so much anymore anyway, and this either
isn't actually a real problem in practice very much anymore - or that
people using raw ALSA still actually do understand it and aren't confused.

"It doesn't work" isn't really a minor problem for the person it doesn't
work for if they can't figure out why.  So I am interested in improving
that in any reasonable way we can.

Going back to the original report again though (rather than the proposed
solutions to it), it does look more like the problem was not in knowing
what didn't work - but rather in knowing what was *expected* to work in
that configuration, and what other options there were to make it work.

A different error message can really only tackle the "what" (non-)problem
part.  If we need to explain "why", that seems like a job for documentation.


  Cheers,
  Ron