On Wed, 20.08.08 16:11, Joe Marcus Clarke ([EMAIL PROTECTED]) wrote:
> + mode = fcntl(out->pcm, F_GETFL);
> + mode &= ~O_NONBLOCK;
> + fcntl(out->pcm, F_SETFL, mode);
Please, please, with cream on top: add some error checking for this.
> +
> + switch (ca_sound_file_get_sample_type(out->file)) {
> + case CA_SAMPLE_U8:
> + val = AFMT_U8;
> + break;
> + case CA_SAMPLE_S16NE:
> + val = AFMT_S16_NE;
> + break;
> + case CA_SAMPLE_S16RE:
> +#if _BYTE_ORDER == _LITTLE_ENDIAN
> + val = AFMT_S16_BE;
> +#else
> + val = AFMT_S16_LE;
> +#endif
> + break;
> + }
> +
> + if (ioctl(out->pcm, SNDCTL_DSP_SETFMT, &val) < 0)
> + goto finish;
> +
> + if (ioctl(out->pcm, SNDCTL_DSP_GETFMTS, &test) < 0)
> + goto finish;
> +
> + if ((val & test) == 0) {
> + errno = EINVAL;
> + goto finish;
> + }
No, this is not the right check. You need to check if val after
calling SNDCTL_DSP_SETFMT is still what you passed
in. SNDCTL_DSP_SETFMT will set the format to what you request, or
something else if that is not supported and return that in val again.
See page 32/33 of the OSS Programmer's guide:
http://www.opensound.com/pguide/oss.pdf
> + val = ca_sound_file_get_nchannels(out->file);
> + if (ioctl(out->pcm, SNDCTL_DSP_CHANNELS, &val) < 0)
> + goto finish;
Again, same thing here. You need to check what val is after calling
the ioctl. And again, since OSS doesn't define any channel maps for
more than 2 chs, you should return NOTSUPPORTED if it is tried to play
a file with more than two channels.
See page 34 in the osspg.
> + val = test = ca_sound_file_get_rate(out->file);
> + if (ioctl(out->pcm, SNDCTL_DSP_SPEED, &val) < 0)
> + goto finish;
> +
> + /* Taken from esound. Check to make sure the configured rate is
> close
> + * enough to the requested rate.
> + */
> + if (fabs((double) (val - test)) > test * 0.05) {
> + errno = EINVAL;
> + goto finish;
> + }
Taking something from esd doesn't really make me feel good ;-) but
this test is actually correct.
> + n_pfd = 2;
> + if (!(pfd = ca_new(struct pollfd, n_pfd))) {
> + ret = CA_ERROR_OOM;
> + goto finish;
> + }
Something else: since you have a fixed number of pollfd structs, you
can allocate them on the stack. Simplifies things. But this doesn't matter,
so if you don't feel like doing this, feel free to ignore this.
Lennart
--
Lennart Poettering Red Hat, Inc.
lennart [at] poettering [dot] net ICQ# 11060553
http://0pointer.net/lennart/ GnuPG 0x1A015CC4
_______________________________________________
libcanberra-discuss mailing list
[email protected]
https://tango.0pointer.de/mailman/listinfo/libcanberra-discuss
