On Wed, Feb 27, 2008 at 8:50 AM, Clemens Ladisch <clemens@xxxxxxxxxx> 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.
>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);
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);
[...]
HTH,
--
Aldrin Martoq
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
