Comment 176 for bug 371897

Revision history for this message
In , Theycallhimart (theycallhimart) wrote :

At last, comments on the code itself :D

For the second example cited from wine/dlls/winepulse.drv/dsoutput.c I'm not sure what you are trying to show, it looks alright to me. The first instance That->buffer = pa_xmalloc0(That->buffer_length) allocates the buffer through the libpulse function pa_xmalloc0 which initializes That->buffer to 0 and in the case off OOM terminates the application. Should HeapAlloc be used in this instance instead? As for the HeapFree() calls on That->buffer and *ippdsdb (which is the value of That), those calls are only reached on error:

    return DS_OK;

err:
    pa_threaded_mainloop_lock(PULSE_ml);
    if (That->stream) {
 if (pa_stream_get_state(That->stream) == PA_STREAM_READY)
     pa_stream_disconnect(That->stream);
 pa_stream_unref(That->stream);
 That->stream = NULL;
    }
    pa_threaded_mainloop_unlock(PULSE_ml);
    if (That->buffer)
 HeapFree(GetProcessHeap(), 0, That->buffer);

    if (*ippdsdb)
 HeapFree(GetProcessHeap(), 0, That);
    *ippdsdb = NULL;
    WARN("exiting with failure.\n");
    return ret;
}

For the first example, the answer is more difficult. It is very hard to place the play pointer, so we set it to be just lagging the write cursor by how much we commit at a time to pulse. The location of This->buffer_read_offset is always correct. It is always a multiple of the fragment size, just like it is for WaveOut Dsound HEL.

I'm waiting until WaveIn support works better before submitting the patches for inclusion because I think that is the right thing to do. Comments? As an aside note, I endorse the fedora packages, if that means anything ;)

Please keep this bug report on topic: better interaction between pulseaudio and wine. You cannot believe that all users will stop using pulseaudio just for the sake of wine. In the long run as pulse becomes and more common and most all applications support it, as current trends suggest, having to manually deal with killing pulseaudio for wine apps to get sound will not look bad for pulseaudio but for wine as it will not "just work." Please be constructive. Commenting on the code is helpful, discrediting the intelligence of people trying the patches is not.