Re: RFR: 8271603: Unnecessary Vector usage in java.desktop [v2]

2021-08-24 Thread Phil Race
On Tue, 24 Aug 2021 21:13:57 GMT, Andrey Turbanov wrote: >> Usage of thread-safe collection `Vector` is unnecessary. It's recommended to >> use `ArrayList` if a thread-safe implementation is not needed. In >> post-BiasedLocking times, this is gets worse, as every access is >> synchronized. >>

Re: RFR: 8271603: Unnecessary Vector usage in java.desktop

2021-08-09 Thread Phil Race
On Sun, 4 Jul 2021 20:42:41 GMT, Andrey Turbanov wrote: > Usage of thread-safe collection `Vector` is unnecessary. It's recommended to > use `ArrayList` if a thread-safe implementation is not needed. In > post-BiasedLocking times, this is gets worse, as every access is synchronized. > I checke

Re: RFR: 8271603: Unnecessary Vector usage in java.desktop

2021-08-09 Thread Phil Race
On Tue, 6 Jul 2021 11:32:18 GMT, Сергей Цыпанов wrote: >> It's not a public API. As I see from other PR/commits changing >> package-private methods shouldn't be a problem. > > Even non-public method can be called via reflection, so I'd be cautios about > changing return type Apps should not b

Re: RFR: 8266421: Deadlock in Sound System [v2]

2021-06-05 Thread Phil Race
On Fri, 4 Jun 2021 02:04:29 GMT, Sergey Bylokhov wrote: >> In the fix for JDK-8207150 I have updated the synchronization of some code >> paths under one "lock", before that code was synchronized only on some >> threads and missing on others. Old review: >> http://mail.openjdk.java.net/pipermail

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v4]

2021-05-27 Thread Phil Race
On Mon, 24 May 2021 13:53:34 GMT, Weijun Wang wrote: >> Please review this implementation of [JEP >> 411](https://openjdk.java.net/jeps/411). >> >> The code change is divided into 3 commits. Please review them one by one. >> >> 1. >> https://github.com/openjdk/jdk/commit/576161d15423f58281e38

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

2021-05-21 Thread Phil Race
On Wed, 19 May 2021 13:47:53 GMT, Weijun Wang wrote: >> Please review this implementation of [JEP >> 411](https://openjdk.java.net/jeps/411). >> >> The code change is divided into 3 commits. Please review them one by one. >> >> 1. >> https://github.com/openjdk/jdk/commit/576161d15423f58281e38

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

2021-05-21 Thread Phil Race
On Thu, 20 May 2021 07:06:00 GMT, Alan Bateman wrote: >> The JEP isn't PTT for 17 so there's plenty of time isn't there ? > > There are 827 files in this patch. Phil is right that adding SW at the class > level is introducing technical debt but if addressing that requires > refactoring and re-t

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

2021-05-19 Thread Phil Race
On Thu, 20 May 2021 04:05:23 GMT, Phil Race wrote: >> By converting JDK-8267432 to a bug, there is an extra benefit that we can >> fix it after RDP. So I'll convert it now. > > That is unfortunate, but nonetheless I think required to be done. > We have acknowledege

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

2021-05-19 Thread Phil Race
On Thu, 20 May 2021 02:09:57 GMT, Weijun Wang wrote: >> I can make it a bug. >> >> I don't want to do it here is because it involves indefinite amount of >> manual work and we will see everyone having their preferences. The more time >> we spend on this PR the more likely there will be merge c

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

2021-05-19 Thread Phil Race
On Wed, 19 May 2021 22:14:20 GMT, Weijun Wang wrote: >> I don't think it is a separate P4 enhancement (?) that someone will (maybe) >> do next release. >> I think it should all be taken care of as part of this proposed change. > > I just made it P3 (P4 was the default value), and I will target i

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

2021-05-19 Thread Phil Race
On Wed, 19 May 2021 21:53:35 GMT, Weijun Wang wrote: >> That's a sad limitation of the annotation stuff then, but I don't think that >> it is insurmountable. >> You can define a static private method to contain this and call it from the >> static initializer block. >> Much better than applying

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

2021-05-19 Thread Phil Race
On Wed, 19 May 2021 18:38:39 GMT, Weijun Wang wrote: >> src/java.desktop/share/classes/java/awt/Component.java line 217: >> >>> 215: * @author Sami Shaio >>> 216: */ >>> 217: @SuppressWarnings("removal") >> >> Why is this placed on the *entire class* ?? >> This class is 10535 lines long

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

2021-05-19 Thread Phil Race
On Wed, 19 May 2021 13:47:53 GMT, Weijun Wang wrote: >> Please review this implementation of [JEP >> 411](https://openjdk.java.net/jeps/411). >> >> The code change is divided into 3 commits. Please review them one by one. >> >> 1. >> https://github.com/openjdk/jdk/commit/576161d15423f58281e38

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

2021-05-19 Thread Phil Race
On Wed, 19 May 2021 13:47:53 GMT, Weijun Wang wrote: >> Please review this implementation of [JEP >> 411](https://openjdk.java.net/jeps/411). >> >> The code change is divided into 3 commits. Please review them one by one. >> >> 1. >> https://github.com/openjdk/jdk/commit/576161d15423f58281e38

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

2021-05-19 Thread Phil Race
On Wed, 19 May 2021 13:47:53 GMT, Weijun Wang wrote: >> Please review this implementation of [JEP >> 411](https://openjdk.java.net/jeps/411). >> >> The code change is divided into 3 commits. Please review them one by one. >> >> 1. >> https://github.com/openjdk/jdk/commit/576161d15423f58281e38

Re: RFR: 8266248: Compilation failure in PLATFORM_API_MacOSX_MidiUtils.c with Xcode 12.5

2021-04-30 Thread Phil Race
On Fri, 30 Apr 2021 05:13:14 GMT, Sergey Bylokhov wrote: > The new XCode does not like the cast of NULL to the UInt32. The fix replaces > NULL with zero. Marked as reviewed by prr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3808

Re: RFR: 8265486: ProblemList javax/sound/midi/Sequencer/Recording.java on macosx-aarch64

2021-04-19 Thread Phil Race
On Mon, 19 Apr 2021 21:33:36 GMT, Mikael Vidstedt wrote: > The javax/sound/midi/Sequencer/Recording.java (tier3) test fails > intermittently on macosx-aarch64. Let's problem list it for now. Marked as reviewed by prr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3579

Re: RFR: 8189198: Add "forRemoval = true" to Applet API deprecations [v2]

2021-03-25 Thread Phil Race
On Thu, 25 Mar 2021 22:58:53 GMT, Andy Herrick wrote: >> implementation of >> JDK-8256145: JEP 398: Deprecate the Applet API for Removal > > Andy Herrick has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > bro

Re: RFR: 8262161 Refactor manual I/O stream copying to new convinient methods in java.desktop

2021-02-26 Thread Phil Race
On Fri, 26 Feb 2021 20:21:21 GMT, Sergey Bylokhov wrote: >> src/java.desktop/unix/classes/sun/print/UnixPrintJob.java line 601: >> >>> 599: try (BufferedInputStream bin = new >>> BufferedInputStream(instream); >>> 600: BufferedOutputStream bout = new >>> BufferedOu

Re: RFR: 8262161 Refactor manual I/O stream copying to new convinient methods in java.desktop

2021-02-26 Thread Phil Race
On Mon, 21 Dec 2020 07:54:17 GMT, Andrey Turbanov wrote: > Cleanup code to use new handy methods in > `java.io.InputStream`/`java.nio.file.Files` instead of manual stream copy: > 1. java.io.InputStream#readAllBytes > 2. java.io.InputStream#transferTo > 3. java.nio.file.Files#copy > > Similar i

Re: RFR: 8262161 Refactor manual I/O stream copying to new convinient methods in java.desktop

2021-02-25 Thread Phil Race
On Mon, 21 Dec 2020 07:54:17 GMT, Andrey Turbanov wrote: > Cleanup code to use new handy methods in > `java.io.InputStream`/`java.nio.file.Files` instead of manual stream copy: > 1. java.io.InputStream#readAllBytes > 2. java.io.InputStream#transferTo > 3. java.nio.file.Files#copy > > Similar i

Re: RFR: 8080272 Refactor I/O stream copying to use java.io.InputStream.transferTo

2020-12-20 Thread Phil Race
On Sun, 20 Dec 2020 20:22:48 GMT, Andrey Turbanov wrote: >> jrtfs is compiled twice, the second is to --release 8 so it can be packaged >> into jrt-fs.jar for use by IDEs/tools running on older JDK releases. So need >> to be careful with the changes here as it will likely causing build breakag

Re: RFR: 8258006: Replaces while cycles with iterator with enhanced for in java.desktop [v2]

2020-12-16 Thread Phil Race
On Tue, 15 Dec 2020 15:24:03 GMT, Andrey Turbanov wrote: >> There are few places in code where manual `while` loop is used with >> `Iterator` to iterate over `Collection`. >> Instead of manual `while` cycles it's preferred to use _enhanced-for_ cycle >> instead: it's less verbose, makes code e

Re: RFR: 8258006: Replaces while cycles with iterator with enhanced for in java.desktop

2020-12-15 Thread Phil Race
On Tue, 15 Dec 2020 17:14:11 GMT, Andrey Turbanov wrote: >>> I've added few words in description. >>> >>> About testing: as I can see testing is done via Github Actions. tier1 >>> builds are ok. >> >> OK that's better But about testing. The github actions won't run a single >> test that touc

Re: RFR: 8258006: Replaces while cycles with iterator with enhanced for in java.desktop

2020-12-14 Thread Phil Race
On Mon, 14 Dec 2020 19:48:36 GMT, Andrey Turbanov wrote: > I've added few words in description. > > About testing: as I can see testing is done via Github Actions. tier1 builds > are ok. OK that's better But about testing. The github actions won't run a single test that touches any code with

Re: RFR: 8258006: Replaces while cycles with iterator with enhanced for in java.desktop

2020-12-12 Thread Phil Race
On Wed, 9 Dec 2020 23:50:23 GMT, Sergey Bylokhov wrote: >> 8258006: Replaces while cycles with iterator with enhanced for in >> java.desktop > > The bug is filed: > https://bugs.openjdk.java.net/browse/JDK-8258006 Whilst this looks "reasonable" I am not impressed by the complete lack of descri

Re: RFR: 8253322 : Update the specification in the newly added constructors

2020-09-23 Thread Phil Race
On Wed, 23 Sep 2020 07:03:59 GMT, Sergey Bylokhov wrote: > Looks like different people use a different style for the new constructors, > it will be good to unify them. > > The text "Constructor for subclasses to call." should be used in the > protected constructors in the abstract classes > In

Re: RFR: 8253322 : Update the specification in the newly added constructors

2020-09-23 Thread Phil Race
On Wed, 23 Sep 2020 07:03:59 GMT, Sergey Bylokhov wrote: > Looks like different people use a different style for the new constructors, > it will be good to unify them. > I have started from https://bugs.openjdk.java.net/browse/JDK-8252721 > See > https://bugs.openjdk.java.net/browse/JDK-8252908?

Re: RFR: 8238738 AudioSystem.getMixerInfo() takes about 30 sec to report a gone audio device

2020-02-10 Thread Phil Race
+1 -phil On 2/9/20 11:03 PM, Sergey Bylokhov wrote: Hello. Please review the fix for jdk/client. Bug: https://bugs.openjdk.java.net/browse/JDK-8238738 Fix: http://cr.openjdk.java.net/~serb/8238738/webrev.00 I propose to decrease the constant "WAIT_BETWEEN_CACHE_REFRESH_MILLIS" and update the

Re: [15] Review Request: 8236980 Cleanup of toString methods in JavaSound

2020-01-22 Thread Phil Race
http://cr.openjdk.java.net/~serb/8236980/webrev.00/src/java.desktop/share/classes/javax/sound/midi/MidiDevice.java.sdiff.html - * Provides a string representation of the device information. + * Returns a string representation of the info object. * - * @return a description of the info o

Re: [13] Review Request: 8222083 Support of "64-bit IEEE floating point" encoding for the AU file format

2019-06-07 Thread Phil Race
No. -Phil. > On Jun 7, 2019, at 6:38 PM, Sergey Bylokhov > wrote: > > Hi, Phil. > >> On 16/04/2019 15:13, Sergey Bylokhov wrote: >> As far as I understand we usually described the "audio file formats" itself >> like AU, WAV, etc. >> But we do not describe which "audio formats" are supported

Re: [PATCH] Fix some assignments of string literals to LPSTR (instead of LPCSTR)

2019-05-16 Thread Phil Race
First thing you need is a bug : https://bugs.openjdk.java.net/browse/JDK-8224056 Second is a sponsor. I can help there. Can you resubmit with your patch in-line (not an attachment) and the subject line: RFR: 8224056: Fix some assignments of string literals to LPSTR (instead of LPCSTR) You sa

Re: [13] Review Request: 8222083 Support of "64-bit IEEE floating point" encoding for the AU file format

2019-04-16 Thread Phil Race
I was first just trying to establish if we have a precedent for describing / documenting what we support. My reading of the CSR is that we generally don't since you call it implementation. So good to at least document these things in a CSR, but is that as far as it should go ? -phil. On 4/1

Re: RFR: JDK-8215217: OpenJDK Source Has Too Many Swear Words

2018-12-11 Thread Phil Race
1) Thanks for uploading the webrev. much better for one person to do this than make everyone who wants to look at it go through a tedious and off-putting set of steps. 2) I've added some client lists since you are touching UI client files, not just core-libs.    To me the client ones look OK,

Re: [12] Review Request: 8207150 Clip.isRunning() may return true after Clip.stop() was called

2018-08-03 Thread Phil Race
I don't know what your notation for the race condition means. Please expand and explain in more detail how this happens and add it to the bug report as well as here. -Phil. > On Aug 3, 2018, at 2:14 PM, Sergey Bylokhov > wrote: > > Hello, Audio Guru. > > Please review the fix for jdk12. >

Re: [11] Review Request: 8202264 Race condition in AudioClip.loop()

2018-07-09 Thread Phil Race
Looks ok. Are you sure the test does not need to be marked as headful ? I'm OK if you need to make that change to not send an updated webrev. -phil. On 07/09/2018 04:25 AM, Sergey Bylokhov wrote: Hello, Audio Guru. Please review the fix for jdk11. Bug: https://bugs.openjdk.java.net/browse/JDK

Re: [11] Review Request: 8205456 Unification of iterations over arrays

2018-06-21 Thread Phil Race
Assuming Dan gets a satisfactory answer, this is OK by me. -phil. On 06/21/2018 02:44 PM, Dan Rollo wrote: Hi Sergey, Nifty. The “Arrays.stream” approach handles the case where the passed in parameter is null? Assuming, yes, then looks good to me. Dan On Jun 20, 2018, at 10:27 PM, Sergey B

Re: [11] Review Request: 8204454 Remove of sun.applet.AppletAudioClip

2018-06-12 Thread Phil Race
+1 -phil. On 06/09/2018 03:49 PM, Sergey Bylokhov wrote: Hello, Audio Guru. Please review the fix for jdk11. Bug: https://bugs.openjdk.java.net/browse/JDK-8204454 Webrev: http://cr.openjdk.java.net/~serb/8204454/webrev.00 The sun.applet.AppletAudioClip class is a part of implementation of A

Re: [11] Review Request: 8201279 javax.sound tests should not set java.home system property

2018-05-14 Thread Phil Race
I had a similar though as Roger on the naming and documentation-level of the property but I suppose there is no problem and there is precedent. I've reviewed the CSR. +1 -phil. On 05/04/2018 06:03 PM, Sergey Bylokhov wrote: CSR is created: https://bugs.openjdk.java.net/browse/JDK-8202680 On

Re: [11] Review Request: 8202050 Add javax/sound/midi/Sequencer/Recording.java to the problemList

2018-04-20 Thread Phil Race
+1 -phil. On 04/19/2018 08:48 PM, joe darcy wrote: Looks fine Sergey; thanks, -Joe On 4/19/2018 5:19 PM, Sergey Bylokhov wrote: Hello, Audio Guru. Please review the fix for jdk11. Bug: https://bugs.openjdk.java.net/browse/JDK-8202050 The "Recording.java" test is added to the ProblemList.

Re: [11] Review Request: 4912693 Behavior of null arguments not specified in Java Sound

2018-03-19 Thread Phil Race
looks good + I have reviewed the CSR. -phil. On 03/17/2018 11:16 PM, Sergey Bylokhov wrote: Hi, Phil. Thank you for review. An updated version: http://cr.openjdk.java.net/~serb/4912693/webrev.02 CSR: https://bugs.openjdk.java.net/browse/JDK-8199763 On 16/03/2018 08:57, Phil Race wrote

Re: [11] Review Request: 4912693 Behavior of null arguments not specified in Java Sound

2018-03-16 Thread Phil Race
- * are examples of typical and acceptable run time exceptions for such cases. + * are the examples of typical and acceptable run time exceptions for such + * cases. all changes like this make the grammar WRONG. "the examples" means you've enumerated all of them in which case they are no longe

Re: [OpenJDK 2D-Dev] RFR: JDK-8071469 Cleanup include and exclude of sound native libraries after source code restructure

2018-03-15 Thread Phil Race
one that matters. -phil. On 03/15/2018 12:06 PM, Alex Menkov wrote: On 03/15/2018 11:44, Magnus Ihse Bursie wrote: On 2018-03-15 18:23, Phil Race wrote: I wondered if that might be the case since it was a "BSD" port .. using X11 .. Maybe we should be getting rid of them ? I agree

Re: [OpenJDK 2D-Dev] RFR: JDK-8071469 Cleanup include and exclude of sound native libraries after source code restructure

2018-03-15 Thread Phil Race
d. /Erik On 2018-03-15 09:53, Phil Race wrote: It is very hard to follow all the moved around files, but one thing that sticks out is there is a "bsd" directory created and I can't work out how the files in there are used. If they are for a BSD port of OpenJDK where is rest of t

Re: [OpenJDK 2D-Dev] RFR: JDK-8071469 Cleanup include and exclude of sound native libraries after source code restructure

2018-03-15 Thread Phil Race
It is very hard to follow all the moved around files, but one thing that sticks out is there is a "bsd" directory created and I can't work out how the files in there are used. If they are for a BSD port of OpenJDK where is rest of the support for that ? On 03/15/2018 07:20 AM, Erik Joelsson wro

jdk10/client forest is FROZEN until JDK 10 repos are consolidated.

2017-09-05 Thread Phil Race
The JDK 10 repo consolidation [1] is about to begin. All the JDK forests need to be frozen to make this happen .. as per this excerpt from the email below //>/We're aiming to have the final pre-consolidation integrations of the />/hs and client forests into JDK 10 master the week of August 28 o

Re: JavaSound is not actually usable on Oracle JDK

2017-08-14 Thread Phil Race
There is no difference between Oracle JDK and OpenJDK Java Sound sources. -phil. On 08/14/2017 06:24 AM, Stefan Reich wrote: Hi there, I am currently using JavaSound on Oracle JDK 1.8 on Linux and Windows. It's an unmitigated catastrophe. Something as simple as playing a Clip only works 60%

Re: [10] Review Request: 8181566 JavaSound javadoc clarification

2017-07-05 Thread Phil Race
http://cr.openjdk.java.net/~serb/8181566/webrev.00/src/java.desktop/share/classes/javax/sound/sampled/AudioSystem.java.sdiff.html 1338 * No default are specified, Either are -> is, or "default" -> "defaults" http://cr.openjdk.java.net/~serb/8181566/webrev.00/src/java.desktop/share/classes/javax

Re: [10] Review Request: 8178403 DirectAudio in JavaSound may hang and leak

2017-07-05 Thread Phil Race
ok. +1 -phil. On 07/05/2017 11:06 AM, Sergey Bylokhov wrote: thread is assigned on creation and we are checking it in run() so (thread == curThread) seems like it must be true until somehow implClose() is called : So why not just check if (thread == null) instead ? This is because we assign t

Re: [10] Review Request: 8178403 DirectAudio in JavaSound may hang and leak

2017-07-05 Thread Phil Race
This is a tricky piece of code with loops, thread comparison checks, a volatile variable, synchronized blocks with monitors and I don't find it easy to review without a better understanding of all of how all of these parts are used here. thread is assigned on creation and we are checking it in ru

Re: [9] Review Request: 8180326 Update the tables in java.desktop to be HTML-5 friendly

2017-05-17 Thread Phil Race
they can read it, but I don't get how making it visible matters to them so how it making it visible relates to accessibility requirements is not an obvious connection to me. So why do we have to make it visible for ATs ? -phil. On 05/17/2017 11:54 AM, Phil Race wrote: I will leave th

Re: [9] Review Request: 8180326 Update the tables in java.desktop to be HTML-5 friendly

2017-05-17 Thread Phil Race
.xml module in the jaxp repo ... pretty much all tables there now have a reasonable, visible caption. -- Jon On 05/17/2017 11:19 AM, Phil Race wrote: I am not sure we are using the summary in a way that makes it worthwhile. As you noted in the other mail "The summary attribute was

Re: [OpenJDK 2D-Dev] [9] Review Request: 8180326 Update the tables in java.desktop to be HTML-5 friendly

2017-05-17 Thread Phil Race
I've looked through all the files now. No other comments. So approved. -phil. On 05/17/2017 11:19 AM, Phil Race wrote: I am not sure we are using the summary in a way that makes it worthwhile. As you noted in the other mail "The summary attribute was used to give a more descriptiv

Re: [9] Review Request: 8180326 Update the tables in java.desktop to be HTML-5 friendly

2017-05-17 Thread Phil Race
I am not sure we are using the summary in a way that makes it worthwhile. As you noted in the other mail "The summary attribute was used to give a more descriptive value of the contents of the table. A caption is more like a title" The values I see are more like a title and as you say that is

Re: [9] Review Request: 8179596 Update java.desktop to be HTML-5 friendly

2017-05-05 Thread Phil Race
This all looks OK to me. -phil. On 05/04/2017 06:15 PM, Sergey Bylokhov wrote: Hello, Please review the fix for jdk9-dev. This fix is a part of the effort to make all javadoc in jdk9 be compatible to HTML5. In the fix the most common issues are fixed. The issues related to tables will be fixe

Re: [9] Review Request: 8178383 Validation of FileIO in the tests for JavaSound should be stricter

2017-05-05 Thread Phil Race
+1 -phil. On 04/11/2017 04:30 AM, Sergey Bylokhov wrote: Hello, Please review the fix for jdk9. Bug: https://bugs.openjdk.java.net/browse/JDK-8178383 Webrev can be found at: http://cr.openjdk.java.net/~serb/8178383/webrev.01 This fix m

Re: [9] Review Request: 8178971 Uncommon formatting and typos in java.desktop module

2017-04-19 Thread Phil Race
+1 -phil. On 04/19/2017 10:29 AM, Sergey Bylokhov wrote: Hello, Please review the fix for jdk9. Bug: https://bugs.openjdk.java.net/browse/JDK-8178971 Webrev can be found at: http://cr.openjdk.java.net/~serb/8178971/webrev.00 Webrev which

Re: RFR: 8178685: Update links to guide in javax sound package javadoc

2017-04-17 Thread Phil Race
22:57, Phil Race написал(а): Bug: https://bugs.openjdk.java.net/browse/JDK-8178685 Webrev : http://cr.openjdk.java.net/~prr/8178685/ The package descriptions for the javax/sound packages reference a relative link which is not valid in JDK 9 docs. The docs/pub team recommend linking directly to

RFR: 8178685: Update links to guide in javax sound package javadoc

2017-04-12 Thread Phil Race
Bug: https://bugs.openjdk.java.net/browse/JDK-8178685 Webrev : http://cr.openjdk.java.net/~prr/8178685/ The package descriptions for the javax/sound packages reference a relative link which is not valid in JDK 9 docs. The docs/pub team recommend linking directly to the tutorial (which was indirec

Re: [9] Review Request: 8177560 @headful key can be removed from the tests for JavaSound

2017-03-30 Thread Phil Race
OK. So +1 from me. -phil On 03/26/2017 05:56 AM, Sergey Bylokhov wrote: An issue with this maybe that access to the desktop is required in order to access the audio device even if it is there as that is how permissions may be set up to authenticate access and limit it to a single user. In t

Re: [9] Review Request: 6574989 TEST_BUG: javax/sound/sampled/Clip/bug5070081.java fails sometimes

2017-03-22 Thread Phil Race
+1 .. and approved to push per RDP2 rules. -phil. On 03/22/2017 05:48 AM, Sergey Bylokhov wrote: It was added during debugging and unused in final version. Fixed: http://cr.openjdk.java.net/~serb/6574989/webrev.02/ 21 марта 2017 г., в 23:36, Dan Rollo написал(а): Hi Sergey, Minor: Why the

Re: RFR 8172765, closed/javax/sound/sampled/Clip/AppletAudioClip/SoundBug.java failed in headless system

2017-01-13 Thread Phil Race
I deleted core-libs since I can't for the life of me think what this has to do with that list Anyway, "+1" -phil. On 01/13/2017 09:57 AM, Felix Yang wrote: Hi team, please review the patch to mark following test as headful. It has been failing in Mach 5 for several runs Bug: https://bu

Re: [OpenJDK 2D-Dev] RFR(M): 8170798: Fix minor issues in java2d and sound coding.

2016-12-14 Thread Phil Race
+1 to the changes you have here. Comments on the upstream issues in-line. -phil. On 12/14/2016 01:45 AM, Lindenmaier, Goetz wrote: Hi Phil, thanks for looking at my change! Updated webrev: http://cr.openjdk.java.net/~goetz/wr16/8170798-java2d_sound/webrev.02/ Best regards, Goetz. (1) I n

Re: [OpenJDK 2D-Dev] RFR(M): 8170798: Fix minor issues in java2d and sound coding.

2016-12-13 Thread Phil Race
Hi Goetz, Comments in-line but first some general points. (1) I notice you prepared the webrev against jdk9/dev. It should be prepared against jdk9/client - which is where it should also be pushed. (2) However most of these changes are in a 3rd party imported library. We don't edit

Re: [9] Review Request: 8169332 The fix JDK-8083664 in AudioFileWriter can be reverted

2016-11-08 Thread Phil Race
+1 -phil On 11/08/2016 04:49 AM, Alex Menkov wrote: Looks good to me. --alex On 07.11.2016 20:21, Sergey Bylokhov wrote: Hello, Audio Guru. Please review the fix for jdk9. The fix contains two changes: - The fix for JDK-8083664[1] was reverted. Because javadoc works properly w/o it. - Th

RFR: 8155874: Fix java.desktop deprecation warnings about Class.newInstance

2016-11-07 Thread Phil Race
bug: https://bugs.openjdk.java.net/browse/JDK-8155874 Webrev: http://cr.openjdk.java.net/~prr/8155874/ This hits all across the desktop module, hence the cross-post. The Class.newInstance() has been deprecated since it may throw checked exceptions that are not declared. Class.getConstructor().n

Re: [9] Review Request: 8168881 javax/sound/sampled/Clip/OpenNonIntegralNumberOfSampleframes.java fails

2016-11-02 Thread Phil Race
+1 -phil On 11/02/2016 09:56 AM, Sergey Bylokhov wrote: Hello, Audio Guru. Please review the fix for jdk9. The new test(which was added recently) does not take into account that: if there is no any audio device the AudioSystem.getClip() can throw IllegalArgumentException, which is a little

Re: [9] Review Request: 8168998 Incorrect implementation of equals in Encoding and Type in JavaSound

2016-11-01 Thread Phil Race
+1 -phil On 11/01/2016 10:03 AM, Sergey Bylokhov wrote: Hello, Audio Guru. Please review the fix for jdk9. AudioFormat.Encoding(null) and AudioFIleFormat.Type(null) equal to any objects which return null from toString(). Note that behavior of null in these constructors is not specified and

Re: [9] Review Request: 8167028 SunCodec.java can be removed

2016-10-03 Thread Phil Race
+1 -phil. On 10/03/2016 09:29 AM, Alex Menkov wrote: Looks okay to me. --alex On 03.10.2016 17:45, Sergey Bylokhov wrote: Hello, Audio Guru. This is a request to small cleanup of the classes related to FormatConversionProvider. - SunCodec.java is removed. It was used as a parent class for

Re: [9] Review Request: 8160217 JavaSound should clean up resources better

2016-08-22 Thread Phil Race
+1 .. seems like this file "leakage" has been there since this code was introduced. -phil. On 08/22/2016 09:16 AM, Sergey Bylokhov wrote: Hello, Audio Guru. This is a fix for a bug which was found by mach5. A number of tests and one place in jdk, incorrectly releases the files streams for th

Re: [9] Review Request: 8156169 Some sound tests rarely hangs because of incorrect synchronization

2016-05-18 Thread Phil Race
+1 -phil. On 05/18/2016 09:49 AM, Sergey Bylokhov wrote: On 17.05.16 19:15, Phil Race wrote: The reason for the change to tracks.toArray() was not obvious from the code and I understood it only after reading your comment below. So perhaps you could add a short comment before you push. The

Re: [9] Review Request: 8156169 Some sound tests rarely hangs because of incorrect synchronization

2016-05-17 Thread Phil Race
+1. The reason for the change to tracks.toArray() was not obvious from the code and I understood it only after reading your comment below. So perhaps you could add a short comment before you push. -phil. On 05/17/2016 07:30 AM, Sergey Bylokhov wrote: Hello, Audio Guru. Please review the fix f

Re: [9] Review Request: 8156583 Typo in the spec of javax.sound.sampled.AudioFormat.Encoding.toString()

2016-05-09 Thread Phil Race
+1 -phil. On 05/09/2016 01:50 PM, Sergey Bylokhov wrote: Hello, Please review the fix for trivial typo in jdk9. Example in the specification contains the typo: * variable (field) name. For example, {@code PCM_SIGNED.toString()} * returns the name "pcm_signed". Note that the pcm_signed is in l

Re: [9] Review Request: 8156581 Cleanup of ProblemList.txt

2016-05-09 Thread Phil Race
+1 On 05/09/2016 01:29 PM, Sergey Bylokhov wrote: Hello, Please review cleanup for jdk9. Some of the bugs related to the tests of sound/beans in ProblemList.txt were fixed, and can be removed from the list. Two JavaBeans tests still fail, so I created a new JDK-8156579. Bug: https://bugs.op

Re: [9] Review Request: 6729836 JavaSound treats large file sizes as negative and cannot read or skip

2016-04-29 Thread Phil Race
StandardFileFormat as a name doesn't feel quite right but I don't have anything better and its not a big deal. Did you run any JCK tests that might be affected ? If yes, then "+1". >The next step will be to update the writers, but I would like to get an opinions what we should do if the A

Re: CFV: New Sound Group Member: Sergey Bylokhov

2016-03-25 Thread Phil Race
Vote: yes -phil.

CFV: New Sound Group Member: Sergey Bylokhov

2016-03-25 Thread Phil Race
I hereby nominate Sergey Bylokhov to membership in the Sound group. Sergey has been a de facto member of the group for a long time and has contributed many fixes. Votes are due by 9th April 2016. //Only current Members of the Sound Group [1] are eligible to vote on this nomination. For Lazy C

Re: [9] Review Request: 8149958 Implementation/documantation of AudioInputStream.read()/skip() should be updated

2016-03-21 Thread Phil Race
+1 -phil. On 3/17/2016 8:34 AM, Sergey Bylokhov wrote: Hello, Audio Guru. Please review the small fix for jdk9. The read and skip methods in AudioInputStream were implemented in a similar way, both can skip or read only the integral number of frames. But for the skip method(unlike read) this

Re: [9] Review Request: 8151041 Jigsaw-only failure of javax/sound/sampled/spi/AudioFileReader/RecognizeWaveExtensible.java

2016-03-04 Thread Phil Race
+1 -phil. On 03/04/2016 10:59 AM, Sergey Bylokhov wrote: Hello, Please review the small fix for jdk9-jake. In the fix the list in "module java.desktop" is updated, and now it contains the same data as META-INF.services/javax.sound.xxx of the current jdk9-dev. I seems I'll need a sponsor to

Re: [9] Review Request: 8038139 AudioInputStream.getFrameLength() returns wrong value for floating-point WAV

2016-02-05 Thread Phil Race
+1 -phil. On 02/05/2016 09:20 AM, Sergey Bylokhov wrote: Hello, Audio Guru. Please review the fix for jdk9. There are two bugs in this area: - in the WaveExtensibleFileReader.java/WaveFloatFileReader.java we create the AudioInputStream using the data size in bytes instead of the the size in

Re: [9] Review Request: 8147407 Provide support of WaveExtensible sound format

2016-01-14 Thread Phil Race
The description read as if we had some bugs in parsing but reading the webrev, if I understand correctly the problem is just that it was not listed as provider. The rest appears to be just clean up .. If so, approved. -phil. On 1/14/2016 3:25 PM, Sergey Bylokhov wrote: Hello, Audio Guru. P

Re: [9] Review Request: 8064800 AudioSystem/WaveFileWriter can't write PCM_FLOAT, but writes it anyway

2016-01-07 Thread Phil Race
Looks good to me. -phil. On 01/07/2016 12:49 PM, Sergey Bylokhov wrote: Hello, Audio Guru. Please review the fix for jdk9. Support of the PCM_FLOAT encoding was added to the jdk7, but some important things were forgotten: - The new WaveFloatFileWriter.java was not added to the ./services/j

Re: [9] Review Request: 8135088 Typo in AuFileReader

2015-12-22 Thread Phil Race
+1 .. the change was a bit more than I was expecting for bug described as a typo ;-) -phil. On 12/18/2015 04:05 PM, Sergey Bylokhov wrote: Thanks Alex. Any volunteers for a second reviewer? FYI - I just found that our audio_writers still incorrectly handle the "new" PCM_FLOAT format which w

Re: [9] Review Request: 8135160 Endless Loop in RiffReader

2015-09-25 Thread Phil Race
we assume an EOF. On 25.09.15 19:31, Phil Race wrote: I think skip is *supposed* to return >=0 but we should protect against it doing the wrong thing. Treat < 0 as 0 .. or EOF .. -phil. On 09/25/2015 09:22 AM, Sergey Bylokhov wrote: On 25.09.15 19:12, Phil Race wrote: long ret = Ma

Re: [9] Review Request: 8135100 Behavior of null arguments not specified in javax.sound.sampled.spi

2015-09-25 Thread Phil Race
On 09/25/2015 09:43 AM, Sergey Bylokhov wrote: On 25.09.15 19:29, Phil Race wrote: The specification of the sound properties file - and the system property - seems to imply that you can configure javax.sound to use classes that are external to the JDK. So far as I know this has not come up in

Re: [9] Review Request: 8135160 Endless Loop in RiffReader

2015-09-25 Thread Phil Race
I think skip is *supposed* to return >=0 but we should protect against it doing the wrong thing. Treat < 0 as 0 .. or EOF .. -phil. On 09/25/2015 09:22 AM, Sergey Bylokhov wrote: On 25.09.15 19:12, Phil Race wrote: long ret = Math.min(stream.skip(remaining), remaining); what is t

Re: [9] Review Request: 8135100 Behavior of null arguments not specified in javax.sound.sampled.spi

2015-09-25 Thread Phil Race
The specification of the sound properties file - and the system property - seems to imply that you can configure javax.sound to use classes that are external to the JDK. So far as I know this has not come up in looking into things that may need some work for jigsaw. Also the very location of the

Re: [9] Review Request: 8135160 Endless Loop in RiffReader

2015-09-25 Thread Phil Race
long ret = Math.min(stream.skip(remaining), remaining); what is to stop stream.skip returning '-100' ? should you not check for negative ? -phil. On 09/24/2015 04:38 PM, Sergey Bylokhov wrote: Hello. Please review the fix for jdk9. While working on some bugs in javasound, I found one i

Re: JDK 9 RFR of JDK-8134632: Mark javax/sound/midi/Devices/InitializationHang.java as headful

2015-08-27 Thread Phil Race
approved. -phil. On 8/27/15 4:09 PM, joe darcy wrote: Hello, Please review the the small patch below to address: JDK-8134632: Mark javax/sound/midi/Devices/InitializationHang.java as headful When the test javax/sound/midi/Devices/InitializationHang.java is failing (as part of tie

Re: [9] Review Request: 8133677 Specification of AudioFileReader should be clarifed

2015-08-27 Thread Phil Race
+1 -phil. On 08/20/2015 06:01 AM, alexey menkov wrote: Looks good for me. regards Alex On 19.08.2015 18:24, Sergey Bylokhov wrote: Hello. Please review the fix for jdk9. This is specification update of AudioFileReader. CCC request will be filed after technical review. According to specifica

Re: JDK 9 RFR of JDK-8134084: Mark client libs regression tests using randomness

2015-08-20 Thread Phil Race
too. I would prefer that we not label tests just based on grep output. -phil. On 8/20/2015 11:44 AM, joe darcy wrote: Hi Phil, On 8/20/2015 11:14 AM, Phil Race wrote: Joe, How is this keyword interpreted and used ? From the TEST.ROOT file in the jdk/test directory: # The "rando

Re: JDK 9 RFR of JDK-8134084: Mark client libs regression tests using randomness

2015-08-20 Thread Phil Race
Joe, How is this keyword interpreted and used ? How did you select the tests below to be so marked ? I might ask more once I understand the answers to these but looking at just one of these - MTGraphicsAccessTest.java - there is no such thing a spurious failure of this test. If you ever see a fai

Re: [9] Review Request: 8013586 and 8130305

2015-07-24 Thread Phil Race
So .. I suggest to add a comment about where "200" came from. Otherwise OK -phil. On 7/21/15 6:12 PM, Sergey Bylokhov wrote: On 22.07.15 1:25, Phil Race wrote: Is this the issue behind the somewhat cryptic "8130305: AudioSystem behavior depends on order that providers are loca

Re: [9] Review Request: 8013586 and 8130305

2015-07-21 Thread Phil Race
On 07/21/2015 08:17 AM, Sergey Bylokhov wrote: Hello. Please review the fix for jdk9. Two issues were fixed: 8013586: audioInputStream.close() does not release the resource 8130305: AudioSystem behavior depends on order that providers are located Description: - Streams were not closed when nece

Re: RFR: JDK-8074096 Disable (most) native warnings in JDK on a per-library basis

2015-03-04 Thread Phil Race
I like the overall approach. We can then attack the warnings lib by lib and/or platform by platform. I do want to mention that some libs like liblcms are 3rd party open source libraries and it may not always be possible to convince upstream to change their code. -phil. On 03/04/2015 08:30 AM

Request for comment: merging the openjdk client groups ?

2015-02-18 Thread Phil Race
d that might be one consequence worth calling out. Whether to proceed with the idea will depend in part on the feedback to the proposal which we are soliciting here. So is this something we should pursue ? -Phil Race.

Re: [9] Review Request: 8068412 [macosx] Initialization of Cocoa hangs if CoreAudio was initialized before

2015-02-17 Thread Phil Race
Looks fine. No idea if Apple know the old code is broken but we should update to the newer APIs anyway -phil. On 02/09/2015 05:10 AM, Sergey Bylokhov wrote: Hello. Please review a fix for jdk 9. The problem is that NSApplicationDidFinishLaunchingNotification is not coming if we find AudioOutp

Re: Comment on bug JDK-8013586

2015-01-07 Thread Phil Race
Its not clear to me if the bug description is implying an exception was thrown or if there is an additional reason the file cannot be closed. Still, something like what you suggest seems to be needed. The owner of this bug is out until next week so I'll let him comment further after his return.

Re: [9] Review Request: 8058115 Some of MidiDeviceProviders do not follow the specification

2014-09-17 Thread Phil Race
No real problems but I wonder why you made unsupportedDevice declare RuntimeException but actually throw IAE and why you split the test in two ? -phil. On 9/17/14 5:42 AM, Sergey Bylokhov wrote: Hello. Any volunteers? On 10.09.2014 19:46, Sergey Bylokhov wrote: Hello, Please review the fix f

Re: [9] Review Request: 8050852 Javadoc cleanup of javax.sound.midi package

2014-07-23 Thread Phil Race
OK. Approved -phil. On 07/23/2014 12:45 PM, Sergey Bylokhov wrote: On 23.07.2014 23:27, Phil Race wrote: I had started on this but there was a lot to wade through Here http://cr.openjdk.java.net/~serb/8050852/webrev.02/src/share/classes/javax/sound/midi/MidiDevice.java.sdiff.html you

  1   2   >