[jitsi-dev] [jitsi/jitsi-videobridge] Add logs (#276)


#1

You can view, comment on, or merge this pull request online at:

  https://github.com/jitsi/jitsi-videobridge/pull/276

-- Commit Summary --

  * Logs dominant speaker changes at level INFO.
  * Logs received keyframes.

-- File Changes --

    M src/main/java/org/jitsi/videobridge/Conference.java (6)
    M src/main/java/org/jitsi/videobridge/simulcast/SimulcastReceiver.java (25)
    M src/main/java/org/jitsi/videobridge/simulcast/SimulcastStream.java (11)

-- Patch Links --

https://github.com/jitsi/jitsi-videobridge/pull/276.patch
https://github.com/jitsi/jitsi-videobridge/pull/276.diff

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/276


#2

@@ -493,16 +493,12 @@ private void dominantSpeakerChanged()
     {
         Endpoint dominantSpeaker = speechActivity.getDominantEndpoint();

- if (logger.isTraceEnabled())
- {
- logger.trace(
- "The dominant speaker in conference " + getID()
+ logger.info("The dominant speaker in conference " + getID()

We could potentially keep the if statement to guard against unnecessary string concatenation.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/276/files/8ee497779c82129d69847b52128972837a2d8654#r72472466


#3

@@ -399,6 +400,12 @@ public void accepted(RawPacket pkt)
         // timestamps of the RTP packets.
         long pktTimestamp = pkt.getTimestamp();
         boolean frameStarted = false;
+ boolean isKeyFrame = isKeyFrame(pkt);

We could potentially have this inside an if (INFO) statement.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/276/files/8ee497779c82129d69847b52128972837a2d8654#r72472946


#4

@@ -493,16 +493,12 @@ private void dominantSpeakerChanged()
     {
         Endpoint dominantSpeaker = speechActivity.getDominantEndpoint();

- if (logger.isTraceEnabled())
- {
- logger.trace(
- "The dominant speaker in conference " + getID()
+ logger.info("The dominant speaker in conference " + getID()

We don't check for INFO level anywhere else. Would you prefer if I add the check here?

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/276/files/8ee497779c82129d69847b52128972837a2d8654#r72473812


#5

@@ -493,16 +493,12 @@ private void dominantSpeakerChanged()
     {
         Endpoint dominantSpeaker = speechActivity.getDominantEndpoint();

- if (logger.isTraceEnabled())
- {
- logger.trace(
- "The dominant speaker in conference " + getID()
+ logger.info("The dominant speaker in conference " + getID()

I stand corrected. Adding the check.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/276/files/8ee497779c82129d69847b52128972837a2d8654#r72474338


#6

@bgrozev pushed 1 commit.

c2d1aa6 Checks the logger level before trying to log.

···


You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/276/files/8ee497779c82129d69847b52128972837a2d8654..c2d1aa6cf7018337a08901a27004b493d069564f


#7

@bgrozev pushed 1 commit.

09dc368 Only checks whether a packet is a keyframe if logging INFO is enabled.

···


You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/276/files/c2d1aa6cf7018337a08901a27004b493d069564f..09dc368802a505e827f8c48ed7c8c78bd601ce93


#8

@bgrozev pushed 1 commit.

1284318 Prevents NPEs.

···


You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/276/files/09dc368802a505e827f8c48ed7c8c78bd601ce93..12843183d28f216d5277609c3d47bfbe02e489ea


#9

Merged #276.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/276#event-736786796