[alsa-devel] ALSA C++ API updated for 1.0.16

[Aldrin Martoq]

On Wed, Feb 27, 2008 at 8:50 AM, Clemens Ladisch <clemens at ladisch.de> wrote:
> Lasse Kärkkäinen wrote:
>  > > > error(std::string const& function, int err): std::runtime_error("ALSA " + function + " failed: " + std::string(snd_strerror(err))), err(err) {}
>  > > I'm not sure that including the function name and all parameters as they
>  > > appear in the source file in the error message is a good idea; I
>  > > wouldn't want to impose this policy on all applications using this
>  > > header.
>  > It is just an exception thrown, the program may catch it and display
>  > another error message. The original ALSA error code is also included
>  > within it.
>  I'm just objecting to the fact that the function name and ALSA's error
>  string are combined into a string that cannot easily be parsed later.

Many of those strings are not meant to be parsed, they are for human
review (developer/user).


>  > In practice the function name seems to be quite essential for debugging,
>  > as EPIPE or some other of the standard error codes will tell little or
>  > nothing.
>  Agreed.

I would prefer that the method from the C++ API should appear instead
of the internal C API function. The C++ developer is calling the
constructor of alsa::pcm, not snd_pcm_open().

>  > For completeness, I think that it might be desirable to also store the
>  > function name separately (like the code is stored), but can anyone see
>  > any actual use for this?
>  I'd like to have only the snd_strerror() result as the message.  In this
>  case, the function name has to be stored separately if it is needed.

I've found the snd_strerror() messages are quite unusable, at least
for the alsaseq api... they return something like 'file not found'
which seems more related to the internal implementation rather that an
actual alsa-related error...

>  > > > pcm(char const* device = "default", snd_pcm_stream_t stream = SND_PCM_STREAM_PLAYBACK, int mode = 0) {
>  > > This looks too much like a default constructor.  I think neither device
>  > > nor stream should have a default value.
>  > Well, it is a default constructor. One design goal was to make this
>  > wrapper as easy to use as possible, and choosing sensible defaults for
>  > that is helpful.
>  While playback will be used more often, I think the difference between
>  playback and capture is so big that we cannot make one of them the
>  default.

Me too.

[...]

>  > > > ALSA_HPP_CLASS& set_##name(...) { ...; return *this; }\
>  > > Is there a reason why you return the object itself?  Usually, returning
>  > > an object implies that that object is a _different_ object with the
>  > > setting applied, which would mean that the original object was _not_
>  > > changed, but that isn't true here.
>  > This is for function call chaining. It is a common idiom used by the
>  > standard library and others.
>  The STL streams use this to allow using the convert-to-bool operator to
>  check the error status of the stream (because most errors aren't
>  reported by exceptions).  That this also allows chaining is just a side
>  effect.

I found this feature quite useful.

>  > > > hw_config(snd_pcm_t* pcm): pcm(pcm) {
>  > > >   try { current(); } catch (std::runtime_error&) { any(); }
>  > > I don't like using exceptions when there's nothing wrong.  Besides,
>  > > getting the current parameters may fail due to other errors (like device
>  > > unplugged).
>  > It seems like a sensible fallback, anyway. The other option would be to
>  > load any() settings always and have the user call current(), if he wants
>  > those instead. Would you find this preferable?
>  My preference would be that the user has to indicate whether he wants
>  to read the current setting or to begin choosing new settings.  It is
>  not possible for this constructor to guess what the user wants.