[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: [PATCH] winecoreaudio.drv: Improve underrun handling
From:       Ken Thomases <ken () codeweavers ! com>
Date:       2012-01-28 15:57:38
Message-ID: C25ABC7B-1CB5-424C-959D-F78164D5F06E () codeweavers ! com
[Download RAW message or body]

On Jan 27, 2012, at 3:46 PM, Andrew Eikum wrote:

> +    if(list_count(&This->queued_bufinfos) == 0){
> +        sc = AudioQueueGetCurrentTime(This->aqueue, NULL, &req_time, NULL);

This isn't quite the same as the next start time.  Even if there are no remaining \
queued buffers, the queue may still be playing some audio from the last of them.  In \
that case, the queue's current time would not be the actual desired start time of \
this buffer.  Have you verified that the queue schedules it after whatever audio may \
be playing?

Also, what about the fact that AudioQueueGetCurrentTime() seems to return an \
unreliable time when the queue is paused?


> +    if(req_time.mFlags & kAudioTimeStampSampleTimeValid){
> +        sc = AudioQueueEnqueueBufferWithParameters(This->aqueue,
> +                This->public_buffer, 0, NULL, 0, 0, 0, NULL, &req_time, \
> &start_time); +        if(sc != noErr){
> +            OSSpinLockUnlock(&This->lock);
> +            WARN("Unable to enqueue buffer: %lx\n", sc);
> +            return E_FAIL;
> +        }
> +    }else{
> +        sc = AudioQueueEnqueueBufferWithParameters(This->aqueue,
> +                This->public_buffer, 0, NULL, 0, 0, 0, NULL, NULL, &start_time);
> +        if(sc != noErr){
> +            OSSpinLockUnlock(&This->lock);
> +            WARN("Unable to enqueue buffer: %lx\n", sc);
> +            return E_FAIL;
> +        }

I would avoid duplicating those two branches just to change between &req_time or \
NULL.  Either use a pointer variable to hold the argument you're going to pass, \
computed in an "if", or use the conditional ?: operator inline.

-Ken


[prev in list] [next in list] [prev in thread] [next in thread]