New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8240694: [macos 10.15] JavaFX Media hangs on some video files on Catalina #163
Conversation
👋 Welcome back almatvee! A progress list of the required criteria for merging this PR into |
Webrevs
|
/reviewers 2 |
@kevinrushforth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a root cause / evaluation either in JBS or to this PR?
I also noted a few places where it might be helpful to add a comment in the code, so anyone looking at it later will know why you are querying the timestamp later from the event thread.
I'll finish my testing in parallel with your addressing those comments and adding the evaluation.
modules/javafx.media/src/main/java/com/sun/media/jfxmediaimpl/NativeMediaPlayer.java
Show resolved
Hide resolved
modules/javafx.media/src/main/native/jfxmedia/platform/osx/avf/AVFAudioSpectrumUnit.cpp
Outdated
Show resolved
Hide resolved
modules/javafx.media/src/main/native/jfxmedia/platform/osx/avf/AVFMediaPlayer.mm
Outdated
Show resolved
Hide resolved
I updated comments in code and added a root cause / evaluation to JBS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, Tested on MacOS 10.12.6 (Sierra) with the use case provided in JBS).
@sashamatveev This change now passes all automated pre-integration checks. When the change also fulfills all project specific requirements, type
Since the source branch of this PR was last updated there have been 11 commits pushed to the ➡️ To integrate this PR with the above commit message to the |
/integrate |
@sashamatveev The following commits have been pushed to master since your change was applied:
Your commit was automatically rebased without conflicts. Pushed as commit e1cb191. |
https://bugs.openjdk.java.net/browse/JDK-8240694
Progress
Issue
Reviewers
Download
$ git fetch https://git.openjdk.java.net/jfx pull/163/head:pull/163
$ git checkout pull/163