[Request for review and bugid] Gervill - ModelStandardIndexedDirector fails on invalid ranges.

[Joe Darcy]

Andrew John Hughes wrote:
> On 12 April 2010 21:13, Joe Darcy <joe.darcy at oracle.com> wrote:
>   
>> Andrew John Hughes wrote:
>>     
>>> On 12 April 2010 21:07, Joe Darcy <joe.darcy at oracle.com> wrote:
>>>
>>>       
>>>> Andrew John Hughes wrote:
>>>>
>>>>         
>>>>> On 12 April 2010 20:04, Karl Helgason <kalli at midverk.is> wrote:
>>>>>
>>>>>
>>>>>           
>>>>>> Hi,
>>>>>>
>>>>>> I need code review and bugid for the fix:
>>>>>>  http://cr.openjdk.java.net/~kalli/gervill-update2/webrev.01/
>>>>>>
>>>>>> This fixes two issues:
>>>>>>
>>>>>>  * ModelStandardIndexedDirector fails on invalid ranges.
>>>>>>  This fix is very IMPORTANT!
>>>>>>  Some songs+soundfonts combinations fails playing without this fix.
>>>>>>
>>>>>>  * Program change with 16-bit banks doesn't work correctly.
>>>>>>  This is a very minor fix.
>>>>>>
>>>>>> JTreg tests have been created for both those issues.
>>>>>>
>>>>>> I am also working on another fix (not included in this fix)
>>>>>>
>>>>>>  * SoftAbstractResampler doesn't work correctly
>>>>>>  when using interpolation algorithms that requires
>>>>>>  large padding like sinc interpolation.
>>>>>>
>>>>>> regards,
>>>>>> Karl
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>             
>>>>> A slightly off-topic point, but it would be good if this fix had a
>>>>> more descriptive summary than the last when pushed:
>>>>>
>>>>> http://hg.openjdk.java.net/jdk6/jdk6/jdk/rev/74e449a8c951
>>>>>
>>>>> Are these being pushed to JDK7 as well?
>>>>>
>>>>>
>>>>>           
>>>> These Gervill fixes should also be pushed to JDK 7.
>>>>
>>>>
>>>>         
>>> Which tree? I can forwardport the last one now.
>>>
>>>
>>>       
>> I would suggest TL, but as the technical code reviewer, the repo to use is
>> Alexey's call.
>>
>> Thanks,
>>
>> -Joe
>>
>>
>>     
>
> TL was what I was going to suggest until I realised it was a
> sound-related patch.
>
> I gave it a quick try, and it breaks horribly in applying to 7,
> seemingly due to whitespace changes.
>   

Hmm, I thought both copies of Gervill had normalized whitespace.  If 
not, we have scripts to turn a patch for non-normalized sources into a 
patch for normalized sources.

-Joe