[jitsi-dev] [jitsi/jitsi-videobridge] fix(ls-hack): Prevents a swallowed NPE when simulcast is not enabled. (#353)


#1

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

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

-- Commit Summary --

  * fix(ls-hack): Prevents a swallowed NPE when simulcast is not enabled.

-- File Changes --

    M src/main/java/org/jitsi/videobridge/LipSyncHack.java (31)

-- Patch Links --

https://github.com/jitsi/jitsi-videobridge/pull/353.patch
https://github.com/jitsi/jitsi-videobridge/pull/353.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/353


#2

Where is the NPE swallowed ?

···

--
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/353#issuecomment-257927652


#3

@paweldomas In AbstractRTPTranslator:227. We should print an error there.

···

--
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/353#issuecomment-257928374


#4

paweldomas commented on this pull request.

@@ -210,17 +211,37 @@ public void onRTPTranslatorWillWriteAudio(

             return;
         }

+ SimulcastReceiver recv = sourceVC.getTransformEngine()
+ .getSimulcastEngine().getSimulcastReceiver();

···

+
+ Long receiveVideoSSRC;
+ if (recv != null && recv.isSimulcastSignaled())
+ {
+ // FIXME this is a little ugly
+ receiveVideoSSRC = sourceVC.getTransformEngine()

you have "SimulcastReceiver recv" var here already or am I missing something ?

--
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/353#pullrequestreview-6868202


#5

paweldomas commented on this pull request.

@@ -210,17 +211,37 @@ public void onRTPTranslatorWillWriteAudio(

             return;
         }

+ SimulcastReceiver recv = sourceVC.getTransformEngine()
+ .getSimulcastEngine().getSimulcastReceiver();

···

+
+ Long receiveVideoSSRC;
+ if (recv != null && recv.isSimulcastSignaled())
+ {
+ // FIXME this is a little ugly
+ receiveVideoSSRC = sourceVC.getTransformEngine()

the whole logic in the spaghetti here fells like a candidate for a method in the VideoChannel, no ?

--
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/353


#6

@gpolitis pushed 1 commit.

2bdac96 style(ls-hack): Reuses variable.

···


You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/353/files/733b4e3497d36f2192585eb5b7f6d36712f0e3f2..2bdac96f7981c4333f394ff7930b66c45ba6e130


#7

gpolitis commented on this pull request.

@@ -210,17 +211,37 @@ public void onRTPTranslatorWillWriteAudio(

             return;
         }

+ SimulcastReceiver recv = sourceVC.getTransformEngine()
+ .getSimulcastEngine().getSimulcastReceiver();

···

+
+ Long receiveVideoSSRC;
+ if (recv != null && recv.isSimulcastSignaled())
+ {
+ // FIXME this is a little ugly
+ receiveVideoSSRC = sourceVC.getTransformEngine()

Thanks for the suggestion Pawel, I really appreciate it and I agree with you that this looks ugly. I plan to simplify it like this https://github.com/jitsi/jitsi-videobridge/blob/cc/src/main/java/org/jitsi/videobridge/LipSyncHack.java#L213 (please do not pay attention to the unchecked array accesses as the code is not finished yet).

--
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/353


#8

paweldomas approved this pull request.

···

--
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/353#pullrequestreview-6871939


#9

Merged #353.

···

--
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/353#event-845323831