-
Notifications
You must be signed in to change notification settings - Fork 641
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
New webhooks playStart, playStop, recordStart issue #4666 #4738
base: master
Are you sure you want to change the base?
Conversation
🛠 Lift Auto-fixSome of the Lift findings in this PR can be automatically fixed. You can download and apply these changes in your local project directory of your branch to review the suggestions before committing.1 # Download the patch
curl https://lift.sonatype.com/api/patch/github.com/ant-media/Ant-Media-Server/4738.diff -o lift-autofixes.diff
# Apply the patch with git
git apply lift-autofixes.diff
# Review the changes
git diff Want it all in a single command? Open a terminal in your project's directory and copy and paste the following command: curl https://lift.sonatype.com/api/patch/github.com/ant-media/Ant-Media-Server/4738.diff | git apply Once you're satisfied commit and push your changes in your project. Footnotes |
@@ -28,11 +29,12 @@ public void setApplicationContext(ApplicationContext applicationContext) { | |||
|
|||
AppSettings settings = (AppSettings)applicationContext.getBean(AppSettings.BEAN_NAME); | |||
timeoutMS = getTimeoutMSFromSettings(settings, timeoutMS, HLS_TYPE); | |||
|
|||
final AntMediaApplicationAdapter antMediaApplicationAdapter = (AntMediaApplicationAdapter)applicationContext.getBean(AntMediaApplicationAdapter.BEAN_NAME); | |||
|
|||
vertx.setPeriodic(DEFAULT_TIME_PERIOD_FOR_VIEWER_COUNT, yt-> | |||
{ | |||
synchronized (lock) { |
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.
SynchronizeOnNonFinalField: Synchronizing on non-final fields is not safe: if the field is ever updated, different threads may end up locking on different objects.
ℹ️ Learn about @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Command | Usage |
---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
{ | ||
//do not block the thread, run in vertx event queue | ||
vertx.runOnContext(h -> { | ||
|
||
synchronized (lock) { |
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.
SynchronizeOnNonFinalField: Synchronizing on non-final fields is not safe: if the field is ever updated, different threads may end up locking on different objects.
ℹ️ Learn about @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Command | Usage |
---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
@@ -1,13 +1,15 @@ | |||
package io.antmedia.statistic; | |||
|
|||
import io.antmedia.AntMediaApplicationAdapter; | |||
|
|||
public interface IStreamStats { | |||
|
|||
/** | |||
* Register a new viewer to a stream | |||
* @param streamId |
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.
EmptyBlockTag: A block tag (@param, @return, @throws, @deprecated) has an empty description. Block tags without descriptions don't add much value for future readers of the code; consider removing the tag entirely or adding a description.
* @param streamId | |
* |
ℹ️ Learn about @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Command | Usage |
---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
@@ -1,13 +1,15 @@ | |||
package io.antmedia.statistic; | |||
|
|||
import io.antmedia.AntMediaApplicationAdapter; | |||
|
|||
public interface IStreamStats { | |||
|
|||
/** | |||
* Register a new viewer to a stream | |||
* @param streamId | |||
* @param sessionId |
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.
EmptyBlockTag: A block tag (@param, @return, @throws, @deprecated) has an empty description. Block tags without descriptions don't add much value for future readers of the code; consider removing the tag entirely or adding a description.
* @param sessionId | |
* |
ℹ️ Learn about @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Command | Usage |
---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
@@ -539,13 +538,60 @@ public void resetDASHStats(String streamId) { | |||
} | |||
} | |||
|
|||
public void sendStartPlayWebHook(final String streamId, final String viewerId){ | |||
final Broadcast broadcast = getDataStore().get(streamId); |
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.
THREAD_SAFETY_VIOLATION: Unprotected write. Non-private method AntMediaApplicationAdapter.sendStartPlayWebHook(...)
indirectly writes to field this.dataStore
outside of synchronization.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
ℹ️ Learn about @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Command | Usage |
---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
@Override | ||
public void streamPlayItemPlay(ISubscriberStream stream, IPlayItem item, boolean isLive) { | ||
vertx.setTimer(1, l -> getDataStore().updateRtmpViewerCount(item.getName(), true)); | ||
final String streamId = item.getName(); | ||
sendStartPlayWebHook(streamId, getRtmpViewerId()); |
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.
THREAD_SAFETY_VIOLATION: Unprotected write. Non-private method AntMediaApplicationAdapter.streamPlayItemPlay(...)
indirectly writes to field this.dataStore
outside of synchronization.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
ℹ️ Learn about @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Command | Usage |
---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
f3ae736
to
60b0a20
Compare
60b0a20
to
7534766
Compare
@@ -1681,7 +1681,6 @@ private Muxer addMp4Muxer() { | |||
* @return |
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.
EmptyBlockTag: A block tag (@param, @return, @throws, @deprecated) has an empty description. Block tags without descriptions don't add much value for future readers of the code; consider removing the tag entirely or adding a description.
* @return | |
* |
ℹ️ Learn about @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Command | Usage |
---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
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.
@sonatype-lift ignore
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.
I've recorded this as ignored for this pull request.
If you change your mind, just comment @sonatype-lift unignore
.
there is no meaningful code duplication(please review just in case) |
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.
I try to make a quick review and request some changes. Please take a look at the comments. Please make the quality gate pass on sonarcloud and sonatype-lift
69737e7
to
a81c88b
Compare
ad5e723
to
f32454e
Compare
SonarCloud Quality Gate failed. |
Kudos, SonarCloud Quality Gate passed! |
dee675a
to
2681814
Compare
2681814
to
e25041a
Compare
74d3e33
to
6b8211b
Compare
SonarCloud Quality Gate failed. 0 Bugs 85.6% Coverage The version of Java (11.0.15.1) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
#4666
Documentation PR is waiting for this to be merged