On Tue, 7 May 2024 19:53:57 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Alexander Matveev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8282999: Add for support EXT-X-MEDIA tag in HTTP Live Streaming [v2] > > modules/javafx.media/src/main/java/com/sun/media/jfxmedia/locator/HLSConnectionHolder.java > line 480: > >> 478: } >> 479: >> 480: class PlaylistLoader extends Thread { > > Having multiple top-level classes in the same source file is an anti-pattern. > Is there a good reason that these can't be nested static classes? If not, > then please make this change. You might be able to then make the nested > classes private, although there is no harm in leaving them package scope. When these classes were nested classes I got issues with second instance of HLSConnectionHolder. If I remember correctly nested classes of second instance of HLSConnectionHolder were using fields of first HLSConnectionHolder instance. Maybe because I initiated second instance incorrectly. To avoid any such potential issues I decided to move away from nested classes. I would prefer to keep as is or better to move all nested classes under separate package (com.sun.media.jfxmedia.locator.hls). ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1435#discussion_r1593304476