On Sat, 2017-04-22 at 20:37 +0200, Georg Chini wrote: > On 21.04.2017 15:32, Tanu Kaskinen wrote: > > Hi all, > > > > It's been three months since the last release, which means that it's > > time to freeze the master branch in preparation for the next release. > > > > > > Here's a review of the current blockers: > > > > frequent crash in pa_alsa_path_set_volume > > https://bugs.freedesktop.org/show_bug.cgi?id=65520 > > > > Needs investigation, might be difficult to fix. > > > > I took a look into this one and at the log at > https://launchpadlibrarian.net/308478962/pulseverbose.log > > The only possibility it can fail this way is when the channel volumes > of s->real_volume or s->thread_info.current_hw_volume are larger > than PA_VOLUME_MAX / 10. > > There are only two calls to pa_alsa_path_set_volume() and each of > them is preceded by a call to pa_sw_cvolume_divide_scalar(). This > call succeeds, otherwise it would return NULL and > pa_alsa_path_set_volume() would crash at another assertion. > pa_sw_cvolume_divide_scalar() also checks the volumes on entry, > so we can be sure the volumes are valid at that point in time. > A base volume of -60dB corresponds to > s->base_volume = 0.1 * PA_VOLUME_NORM. > > This means that pa_sw_volume_divide() which is called by > pa_sw_cvolume_divide_scalar() roughly multiplies the input channel > values by 10, which leads to the statement above. > > Can we simply clip pa_sw_volume_divide() at PA_VOLUME_MAX? > > Or will we need further investigation? I wouldn't add clipping to pa_sw_volume_divide(). In principle the function should apply clipping for correctness, but if it has to clip, the volume value is humongous, so big that it indicates a bug somewhere. 0.1 * PA_VOLUME_MAX corresponds to about 211 dB. If your analysis is correct, and the input volume is indeed over 200 dB, then I think we should try to figure out how such volume could end up being used. It looks like this happens during initialization, so the initialization code should be reviewed. If no bugs get catched by reviewing, I think we should add some extra logging and/or assertions somewhere so that we have more clues when this happens again. -- Tanu https://www.patreon.com/tanuk
