[MPlayer-dev-eng] [PATCH] ao_dart

[Reimar Döffinger]

On Mon, Feb 16, 2009 at 10:17:01PM +0900, KO Myung-Hun wrote:
> +static volatile int m_nBufSize = 0;
> +static volatile int m_nBufLen = 0;
> +static volatile int m_iBufPos = 0;

You are writing m_nBufLen from both threads, thus your code is not
thread-safe.
E.g. ao_jack and libavutil/fifo.c should be thread-safe implementations
for this one reader, one writer case (or you can use locking), ao_jack
is a bit better optimized for this specific use.
They also do not need modulus-operation which you use and can be quite slow.

> +        case AOCONTROL_GET_VOLUME :
> +        {
> +            ao_control_vol_t *vol = (ao_control_vol_t *)arg;
> +
> +            vol->left = vol->right = LOUSHORT(dartGetVolume());
> +
> +            return CONTROL_OK;
> +        }
> +
> +        case AOCONTROL_SET_VOLUME:
> +        {
> +            int mid;
> +            ao_control_vol_t *vol = (ao_control_vol_t *)arg;

The casts are not necessary.

> +    opt_t subopts[] = {

const

> +        {"noshare", OPT_ARG_BOOL, &fNoShare, NULL},

Use "share" here, will give "share" and "noshare" as possible options,
this will give you "noshare" and "nonoshare".

> +        {"bufsize", OPT_ARG_INT, &nDartSamples, NULL},

I don't think you want to allow negative numbers here, use the
int_pos or int_non_neg test functions (if the later make bufsize = 0
mean the default value.

> +    n = (af_fmt2bits(format) >> 3) * channels;

n is not really a good name.

> +    ao_data.bps         = channels * rate * (af_fmt2bits(format) >> 3);

You could reuse "n" here, too.

> +    // mask off all floating-point exceptions
> +    _control87(MCW_EM, MCW_EM);

Hm. Quite ugly, and you don't restore them in uninit.
Also explaining why they are needed would be good (performance reasons,
crash, and is it a bug in DART or a documented requirement - and no,
no need to research, just write what you know already).