[FFmpeg-devel] [PATCH] Fix the wmavoice-7k fate test (failures on OpenBSD)

[Ronald S. Bultje]

Hi,

On Mon, Aug 9, 2010 at 3:57 AM, Martin Storsj? <martin at martin.st> wrote:
> On Sun, 8 Aug 2010, Ronald S. Bultje wrote:
>> On Sun, Aug 8, 2010 at 8:17 PM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
>> > On Sat, Aug 7, 2010 at 6:12 AM, Martin Storsj? <martin at martin.st> wrote:
>> >> I checked out the wmavoice-7k fate test failures that occurred on OpenBSD,
>> >> and actually got crashes in these tests on Mac OS X, too, under some
>> >> compilation configurations.
>> >>
>> >> I tracked down the issue to aw_pulse_set2, where the loop around lines
>> >> 1071- may write outside of the use_mask buffer if pulse_off starts out
>> >> negative.
>> >>
>> >> The first patch fixes the issue and makes the test pass on OpenBSD, but
>> >> it's a bit ugly. The second patch also fixes the issue, but changes the
>> >> output of the test (but it still sounds the same to me). I believe this is
>> >> the more correct way to fix the issue, but requires the reference sample
>> >> in the testsuite to be updated.
>> >
>> > I think neither are correct. The pulse exclusion length should not be
>> > affected, but the ones with a negative offset should be skipped. Given
>> > that, the second patch is definitely wrong, but the first patch is
>> > probably not right either.
>> >
>> > Since checking length and recalculating length, offset (and so on)
>> > would affect speed significantly, I feel the better (and safer) thing
>> > to do is to pad use_mask with two uint16_ts _before_ the pointer start
>> > (similar to how we pad it with two uint16_ts _after_ currently). That
>> > way, even if idx is the minimum value (-23), it would still write in
>> > valid memory (use_mask[-23 >> 4] = use_mask[-2]). these values should
>> > be initialized to zero, similar to the post-use_mask-pad values. At
>> > the same time, no length checks are necessary, and thus the code is
>> > safe even with negative offset values.
>>
>> As in attached. I'll try to test later, if anyone else can test I'd
>> appreciate it.
>
> Looks ok, and works for me. In the setup where it crashed on OS X before,
> it now works just fine (and all other wmavoice tests still pass, too). I
> unfortunately already scrapped the OpenBSD VM, but I'm quite certain this
> will fix the issue there, too.

Applied, thanks.

Ronald