Bug#578720: vorbis-tools: Sends invalid argument to ALSA when playing some files

[Martin Steghöfer]

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:
> 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.

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!).

> (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)

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).

> 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.)

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.

> 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.

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.

> 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).

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

>> And then there's the risk of breaking other software by changing the error code, like you said. :-(
> Right, including breaking libao if we were to do the trick you proposed
> above :)

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).

> 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.

Unfortunately that's true... ;-(

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