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

[Martin Steghöfer]

Hi Ron!

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

Ah, OK, that's good :-)

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

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

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

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

> Right, but "Set hardware channels failed" doesn't explain it much
> differently.

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

> And "invalid parameters" is all alsa told us.

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

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

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

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

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