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

[Ron]

On Sat, Dec 13, 2014 at 12:23:22PM +0100, Martin Steghöfer wrote:
> Ron wrote:
> >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?

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)
 

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

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


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

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.


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

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