RFR: 8282999: Add support for EXT-X-MEDIA tag in HTTP Live Streaming [v3]

Alexander Matveev almatvee at openjdk.org
Wed May 8 02:33:58 UTC 2024


On Tue, 7 May 2024 19:53:57 GMT, Kevin Rushforth <kcr at 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


More information about the openjfx-dev mailing list