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

[Alex Menkov]

Joe, Andrew,

sound fixes have a bit tangled "patch logistics" :)
Karl pushes changes into openjdk6, then I forwardport them into jdk7 (to 
minimize QE I push sound fixes in batches, so often fixes go to jdk7 
with a delay).
openjdk6 repo does not use jcheck extension, so I fix whitespace 
normalization issues only pushing changes into jdk7.

regards
Alex

Joe Darcy wrote:
> 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