Lasse Kärkkäinen wrote:> Here's an updated version of the C++ wrapper that I posted earlier in> September 2007. I would like this to be included in the ALSA> distribution after some peer review.
Yes, an ALSA C++ wrapper is a good idea.
> 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 theyappear in the source file in the error message is a good idea; Iwouldn't want to impose this policy on all applications using thisheader.
> 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 devicenor stream should have a default value.
> ALSA_HPP_CLASS& set_##name(...) { ...; return *this; }\
Is there a reason why you return the object itself? Usually, returningan object implies that that object is a _different_ object with thesetting applied, which would mean that the original object was _not_changed, but that isn't true here.
> class hw_config: internal::noncopyable {
Why should this object not be copyable?
And why do you have two different objects (*w_params and *w_config) forwrapping the hardware/software parameters?
> 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 deviceunplugged).
I think it would be better if this object doesn't have a publicconstructor, and you would be able to create one only through a pcmobject, like "hw_config c = pcm.any_hw_params();".
> ~mmap() {> // We just assume that this works (can't do anything sensible if it fails).> snd_pcm_mmap_commit(pcm, offset, frames);
This is the place where all the usual write errors must be checked.This cannot be done in a destructor. I think using RAII for the non-error commit just isn't possible.
Regards,Clemens_______________________________________________Alsa-devel mailing listAlsa-devel@xxxxxxxxxxxxxxxxxxxx://mailman.alsa-project.org/mailman/listinfo/alsa-devel
