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

[Lasse Kärkkäinen]

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

As Aldrin pointed out, it is not meant to be parsed. I have now provided
separate functions for accessing the function names directly.

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

"alsa::pcm::pcm: snd_pcm_open failed: No such file or directory" is
useful for debugging, "No such file or directory" is not. Therefore, I
don't see why you want only snd_strerror in that message.

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

I have removed the default arguments, as you requested.

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

I have looked at the issue again; the current system (load current if
possible, any otherwise) is to make it resemble the API that sw_config
provides (always loading the current settings). If the user specifically
wants current or any settings (and wants an exception if that fails), he
may call the any() and current() functions after first constructing the
object. If not, he will get the most sensible default behavior.

However, I changed to catch to only match alsa::error (not that it
matters, but it's clearer that way).

>> Some settings need to be loaded by the constructor in order to ensure
>> that the structure is in a consistent state at all times.
>
> ALSA's snd_pcm_hw_params_t is in a consistent state even before initial
> or current settings are loaded from a PCM device.  But then the
> hw_config object isn't the direct wrapper for snd_pcm_hw_params_t ...

The documentation says that an invalid snd_pcm_hw_params_t is allocated
(by snd_pcm_hw_params_malloc). I did not look into ALSA source code to
find out whether this means uninitialized memory or just a kind of
"none" configuration space that could still be used by the other
functions. In the latter case, removing the initialization from
hw_config constructor, and having the user explicitly call any() or
current(), would be a sensible choice.

In this case it would be nice if this behavior was also clearly
documented on the C API.

> In our case, a rollback is equivalent to calling snd_pcm_mmap_commit
> with frames==0.

Ah, didn't think of that. Fixed to work this way, safe commit function
provided.

> Hmm, it should be possible to commit any number of frames as long as it
> isn't more than originally requested.

One could try to commit more than the original number with the old
wrapper, but this is fixed now (among with the commit function).

> After committing, it is indeed not allowed to access the mmap buffer,
> so this isn't too unexpected.

Just to stay safe, I made it throw std::logic_error if any access is
attempted after commit.

> BTW: The object should be called mmap_access or something like that.

This differs from the naming convention used in other parts of the
wrapper (take a part of the C API function name). It seems pretty
obvious what an object called mmap does on a PCM, so I don't quite see
why you want another name for it.

> As far as I can see, your header tries to do two things: wrapping the
> ALSA API, and offering an easier-to-use API that is not as low-level
> as the ALSA API itself.  I think these two should be put in separate
> header files.

I don't see why they couldn't coexist in the same header. C++
programmers are used to some flexibility (things happening
automatically, function overloading, default arguments, "OOP" syntax),
so it seems normal to offer these in such a wrapper.

However, I don't intend alsa.hpp, as it is now, as an easy solution for
the user, but have instead written a separate audio library on top of
it, abstracting many of the features of ALSA and also supporting other
audio interfaces under the hood.

> Anyway, we need more wrappers for all those data structures.  I'll see
> what I can come up with.

True. The problem is that it is a lot of work, even with all those
macros. Without helper macros one would need thousands of lines just to
wrap all the functions.

Any help is appreciated, of course :)


Aldrin wrote:
> 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().

I have fixed this. Now it prints the calling function, the C API
function and the snd_strerror message. All can be requested as
individual strings to order having to parse the formatted message
returned by what().

> From a OO perspective, I would expect that a *_config object be
> created from an existing pcm object. Something like:
> 
> alsa::pcm pcm;
> 
> pcm.get_config().set(FOO).set(BAR);

This does not allow storing the generated configuration, which is a
requirement if we want to add some control logic to it instead of just
setting things blindly. (remember, hw_config is noncopyable because it
depends on a PCM, which is noncopyable)

> If there is something that doesn't apply to a pcm instance, the C++
> API could provide an instance of a pcm_any object and call that
> instead:
> 
> alsa::pcm_any.get_config().set(FOO).set(BAR);

Even the "any" configuration is always linked to a PCM:
int snd_pcm_hw_params_any(snd_pcm_t *pcm, snd_pcm_hw_params_t *params);

A new version is attached.

Changelog for alsa.hpp 0.6:
- alsa::mmap completely rewritten
- alsa::error now has accessors for function names
- ALSA_CHECKED and alsa::error changed to also store the name of the
function using the macro
- alsa::pcm device and stream default values removed
- catching errors only, instead of any runtime_error
- added missing *_param operator=(*_param...)
- naming style changed: member variables now use m_ prefix to avoid
shadowing warnings from GCC -Wshadow
- other compile warnings eliminated as well (-Weffc++ etc)
- documentation updates

-------------- next part --------------
A non-text attachment was scrubbed...
Name: alsa.hpp
Type: text/x-c++hdr
Size: 23856 bytes
Desc: not available
Url : http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20080301/9a1b2078/attachment-0001.hh