[alsa-devel] oss plugin: SNDCTL_DSP_SETFRAGMENT problems

[Andriy Gapon]

First, a disclaimer: what I'd like to discuss seems to affect a FreeBSD OSS
implementation.

After looking at the code in oss_pointer() and in snd_pcm_ioplug_hw_ptr_update()
it seems that the code depends on OSS driver using exactly the same buffer size
as specified by the buffer_size parameter.
Otherwise the code that handles hw pointer cycling would calculate a delta
incorrectly.

Then, it seems that oss_hw_params() tries to enforce the driver's buffer size
using SNDCTL_DSP_SETFRAGMENT.  But in the OSS specification there are oh so many
warnings about it:
http://manuals.opensound.com/developer/SNDCTL_DSP_SETFRAGMENT.html
> It's important to understand that this call just sets the size hint. There is
> no guarantee that the requested size gets used.
...
> It's very important to make this ioctl call as early as possible after
> opening the device since otherwise the call may not have any effect. In
> particular this call must be made before calling read, write or some other
> ioctl calls.
...
> This ioctl call will return an error only if the argument value is incorrect
> or if the call is made in a wrong place. Even if the call returned OK there
> is no guarantee that the requested buffer size will be used. This ioctl call
> sets just the size hint and the driver may or may not honor the request.

I see at least two issues with the oss plugin code:

o Actual OSS driver buffer size value is never queried/verified if the
SNDCTL_DSP_SETFRAGMENT call is successful

o  Apparently oss_hw_constraint() function is called before oss_hw_params() and
the former makes SNDCTL_DSP_CHANNELS ioctl calls which change a configuration of
the OSS driver and may affect the future SNDCTL_DSP_SETFRAGMENT call.  This also
violates the order described here:
http://manuals.opensound.com/developer/callorder.html

There is a retry loop in oss_hw_params() which closes and re-opens an OSS
device, but that loop is only activated on SNDCTL_DSP_SETFRAGMENT hard failure;
there is no handling for the case where the ioctl succeeds but the resulting
buffer size differs from the requested one.

I think that to nullify any effects of oss_hw_constraint() on the OSS driver
configuration there should be a call to SNDCTL_DSP_RESET at the end of the
function.  Alternatively, there could be a call to this ioctl before
SNDCTL_DSP_SETFRAGMENT.  Or perhaps there could be resets at both places...

Ideally, of course, the OSS plugin should query the exact OSS driver buffer size
and alter the buffer_size parameter accordingly.  But this is a bit more work.

What do you think?
Thank you.
-- 
Andriy Gapon