Skip to content
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

typo in janus_rtcp_incoming_transport_cc() #2677

Closed
yin-zhang opened this issue May 31, 2021 · 1 comment
Closed

typo in janus_rtcp_incoming_transport_cc() #2677

yin-zhang opened this issue May 31, 2021 · 1 comment
Labels

Comments

@yin-zhang
Copy link

yin-zhang commented May 31, 2021

Ran into a couple of typos in janus_rtcp_incoming_transport_cc() in rtcp.c:

			length = (s ? 7 : 14);
			JANUS_LOG(LOG_HUGE, "  [%"SCNu16"] t=status-vector, ss=%s, l=%"SCNu8"\n", num,
				s ? "2-bit" : "bit", length);

should be changed to:

			length = (ss ? 7 : 14);
			JANUS_LOG(LOG_HUGE, "  [%"SCNu16"] t=status-vector, ss=%s, l=%"SCNu8"\n", num,
				ss ? "2-bit" : "bit", length);

That is, s ==> ss. Otherwise the parsing of TWCC packets may get wrong deltas

@atoppi atoppi added the bug label May 31, 2021
@atoppi
Copy link
Member

atoppi commented May 31, 2021

@yin-zhang thanks for reporting.

According to the draft:

3.1.4.  Status Vector Chunk

   A status vector chunk starts with a 1 bit to identify it as a vector
   chunk, followed by a symbol size bit and then 7 or 14 symbols,
   depending on the size bit.

        0                   1
        0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       |T|S|       symbol list         |
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

   chunk type (T):  1 bit A one identifies this as a status vector
               chunk.

   symbol size (S):  1 bit A zero means this vector contains only
               "packet received" (0) and "packet not received" (1)
               symbols.  This means we can compress each symbol to just
               one bit, 14 in total.  A one means this vector contains
               the normal 2-bit symbols, 7 in total.

   symbol list:  14 bits A list of packet status symbols, 7 or 14 in
               total.

So length (either 7 or 14) should be set according to the ss variable (that matches the symbol size S of the specs).
Hopefully not a big issue, since we're not doing much with incoming twcc feedback.

Anyway, I'm fixing this asap.

@atoppi atoppi closed this as completed in 9eeeb38 May 31, 2021
BogdanovKirill pushed a commit to 3dEYE/janus-gateway that referenced this issue Jun 10, 2021
commit 9c9d335
Author: Lionel Nicolas <[email protected]>
Date:   Thu Jun 10 03:04:43 2021 -0400

    Fix streaming plugin mutex unlock when disabling mountpoint (meetecho#2690)

commit 2d83e96
Author: Yurii Cherniavskyi <[email protected]>
Date:   Mon Jun 7 16:02:41 2021 +0300

    Fix SIP plugin unhold request docs typo (meetecho#2688)

commit 2cd0118
Author: August Black <[email protected]>
Date:   Mon Jun 7 01:10:49 2021 -0700

    minor adjustment to the audiobridge docs (meetecho#2687)

commit de2117e
Author: nicolasduteil <[email protected]>
Date:   Tue Jun 1 11:26:29 2021 +0200

    fix: [janus_sip] Fix "call_id" property in "missed_call" events (meetecho#2679)

commit 9eeeb38
Author: Alessandro Toppi <[email protected]>
Date:   Mon May 31 15:57:41 2021 +0200

    Fix status vector parsing for incoming twcc feedbacks (resolves meetecho#2677).

commit 8a25f6e
Merge: d3b39b9 394fb48
Author: Alessandro Toppi <[email protected]>
Date:   Fri May 28 13:29:54 2021 +0200

    Merge pull request meetecho#2675 from kmeyerhofer/actions/fix

    GH Actions, fix variable name

commit d3b39b9
Author: Lorenzo Miniero <[email protected]>
Date:   Fri May 28 11:09:30 2021 +0200

    Fixed race condition in VideoRoom

commit 394fb48
Author: Kurt Meyerhofer <[email protected]>
Date:   Thu May 27 14:52:08 2021 -0600

    Fixes variable name.

commit b45cd37
Author: Lorenzo Miniero <[email protected]>
Date:   Thu May 27 18:31:55 2021 +0200

    Clarify that libnice 0.1.18 is recommended

commit 5757a37
Author: Lorenzo Miniero <[email protected]>
Date:   Thu May 27 17:08:17 2021 +0200

    Spatial audio support in AudioBridge via stereo mixing (meetecho#2446)

commit 161fe7a
Author: Luca Barbato <[email protected]>
Date:   Thu May 27 15:29:01 2021 +0200

    Cleanup avformat-based preprocessors (meetecho#2665)

commit 7b010cd
Author: lucylu-star <[email protected]>
Date:   Tue May 25 17:09:40 2021 +0800

    Fixed broken simulcast support in VideoCall plugin (meetecho#2671)

commit 4ae44a4
Author: nicolasduteil <[email protected]>
Date:   Mon May 24 17:57:34 2021 +0200

    feat: support for custom call-id in subscribe request + add 'call_id' property to subscribe & notify related events (meetecho#2664)

commit 4294f20
Author: Lorenzo Miniero <[email protected]>
Date:   Mon May 24 11:02:48 2021 +0200

    Fixed missing macro when using pthread mutexes (fixes meetecho#2666)

commit f22ab0d
Author: Lorenzo Miniero <[email protected]>
Date:   Wed May 19 12:03:32 2021 +0200

    Fixed warning

commit b3f3f17
Author: Alessandro Toppi <[email protected]>
Date:   Tue May 18 12:10:47 2021 +0200

    Remove duplicated flag for fuzzing coverage.

commit 4a7560c
Author: nu774 <[email protected]>
Date:   Fri May 14 00:26:36 2021 +0900

    janus-pp-rec: support HEVC AP(aggregation packet) (meetecho#2662)

commit 5db4be2
Author: Lorenzo Miniero <[email protected]>
Date:   Wed May 12 17:43:43 2021 +0200

    Fixed out of bounds array access

commit 69f56f4
Author: nicolasduteil <[email protected]>
Date:   Tue May 11 14:36:22 2021 +0200

    feat: support for SUBSCRIBE expiry (Expires header) in sip plugin (meetecho#2661)

commit b047ccf
Author: Lorenzo Miniero <[email protected]>
Date:   Mon May 10 09:33:27 2021 +0200

    Fixed types

commit f8e8c5e
Author: Chris Wiggins <[email protected]>
Date:   Mon May 10 19:26:45 2021 +1200

    RabbitMQ Transport Reconnect Logic (meetecho#2651)

commit 280e8e4
Author: Lorenzo Miniero <[email protected]>
Date:   Fri May 7 12:54:30 2021 +0200

    Add per-participant recording options in AudioBridge to join API as well
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants