Bug#636231: Does set_period_size() really need to be called from driver_initalise()?

[Steven McDonald]

Hi again,

I've tried compiling jackd1 after simply removing the call to
set_period_size() that causes the segfault, rather than modifying
set_period_size() so that it works if driver->engine hasn't been
initialised. For clarity, I did this (with my previous patch NOT
applied):

--- a/drivers/oss/oss_driver.c
+++ b/drivers/oss/oss_driver.c
@@ -1241,8 +1241,6 @@
        driver->sys_in_latency = in_latency;
        driver->sys_out_latency = out_latency;

-       set_period_size(driver, period_size);
-       
        driver->finish = driver_finish;

        if (driver->indev == NULL)

jackd1 still starts without any problems and audio playback is fine. I
haven't tested recording, but I would expect that this logic would have
the same effect (i.e., none) on recording as on playback.

The reported buffer size is still doubled as a result of doubling the
period size:

---
$ /usr/bin/jackd -r -doss -r176400 -p1024 -n2 -w16 -i0
jackd 0.121.0
Copyright 2001-2009 Paul Davis, Stephane Letz, Jack O'Quinn, Torben
Hohn and others.
jackd comes with ABSOLUTELY NO WARRANTY
This is free software, and you are welcome to redistribute it
under certain conditions; see the file COPYING for details

JACK compiled with System V SHM support.
loading driver ..
oss_driver: /dev/dsp : 0x10/2/176400 (4096)
oss_driver: indevbuf 0 B, outdevbuf 4096 B
oss_driver: using barrier mode, (dual thread)
---
$ /usr/bin/jackd -r -doss -r176400 -p2048 -n2 -w16 -i0
jackd 0.121.0
Copyright 2001-2009 Paul Davis, Stephane Letz, Jack O'Quinn, Torben
Hohn and others.
jackd comes with ABSOLUTELY NO WARRANTY
This is free software, and you are welcome to redistribute it
under certain conditions; see the file COPYING for details

JACK compiled with System V SHM support.
loading driver ..
oss_driver: /dev/dsp : 0x10/2/176400 (8192)
oss_driver: indevbuf 0 B, outdevbuf 8192 B
oss_driver: using barrier mode, (dual thread)
---

In fact, reading through oss_driver.c, the set_period_size() function
seems to be mostly a reimplementation of logic that is provided
elsewhere in the driver. Furthermore, if I remove this call to
set_period_size() and then explicitly set driver->engine to NULL in
set_period_size() before using it, I don't get a segfault. The OSS
driver seems to be working without needing to call set_period_size()
from *anywhere*. (There is one other function which calls it, which
seems to only be invoked if one changes the period size during
operation.)

Can anyone more knowledgeable comment on whether it is necessary to
call set_period_size() here, and if not can we simply use the trivial
patch above instead of the original fix I submitted?

Thanks,
Steven.