Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcap/+/32121 )
Change subject: tests: $(BUILT_SOURCES) is not defined
......................................................................
Patch Set 1:
(1 comment)
File tests/Makefile.am:
https://gerrit.osmocom.org/c/osmo-pcap/+/32121/comment/14083d0e_aa0a562a
PS1, Line 24: python-tests:
> you are missing the binary path here?
It's actually a good question what binary.
There are both -client and -server, perhaps both?
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/32121
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcap
Gerrit-Branch: master
Gerrit-Change-Id: I81dc30f6d6e3b25723f00d945c87b9c01c648cbf
Gerrit-Change-Number: 32121
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 29 Mar 2023 19:03:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32117 )
Change subject: tests: $(BUILT_SOURCES) is not defined, depend on osmo-bts-virtual
......................................................................
Patch Set 1:
(1 comment)
File tests/Makefile.am:
https://gerrit.osmocom.org/c/osmo-bts/+/32117/comment/526fbd30_03156739
PS1, Line 35: python-tests:
> $(top_builddir)/src/osmo-bts-virtual/osmo-bts-virtual
I am intentionally not adding dependencies if they're not used directly.
This dependency is added to the `vty-test` target, where it's used explicityly.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32117
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I4ace06e115a2689bde9afd9f99ecee99d796360b
Gerrit-Change-Number: 32117
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 29 Mar 2023 19:02:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: falconia.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32128 )
Change subject: rtp continuous-streaming: fix BFI in the quality-suppressed case
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32128
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Icee0f57be289a0592a0197469432a012d15f224c
Gerrit-Change-Number: 32128
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Comment-Date: Wed, 29 Mar 2023 18:36:35 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: falconia.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32128 )
Change subject: rtp continuous-streaming: fix BFI in the quality-suppressed case
......................................................................
Patch Set 2:
(1 comment)
File src/common/l1sap.c:
https://gerrit.osmocom.org/c/osmo-bts/+/32128/comment/43b3024e_795e31e3
PS1, Line 1661: send_ul_rtp_packet(lchan, fn, bfi, 0);
> I did not realize that passing a NULL pointer was allowed, for some reason I thought it had to be a […]
well if len = 0, then it makes no sense to access to buffer, so it shouldn't expect it to be non-null in that case.
The question is more whether it's fine to have len=0 on that path, but I think it ws the case before right?
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32128
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Icee0f57be289a0592a0197469432a012d15f224c
Gerrit-Change-Number: 32128
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Comment-Date: Wed, 29 Mar 2023 18:36:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: falconia <falcon(a)freecalypso.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/32128
to look at the new patch set (#2).
Change subject: rtp continuous-streaming: fix BFI in the quality-suppressed case
......................................................................
rtp continuous-streaming: fix BFI in the quality-suppressed case
The check for (tch_ind->lqual_cb >= bts->min_qual_norm) in
l1sap_tch_ind() has the intent of suppressing valid-seeming
speech frame output from lower layers when the link quality is
too low; this check is particularly important for FR1 codec
where the intrinsic validity check is only a 3-bit CRC which has
1/8 probability of indicating "correct" when decoding radio noise
during DTXu silence.
However, this check is effectively defeated in the current
implementation of rtp continuous-streaming, at least when
themyscira-bfi extension is not used or when it doesn't apply
because the codec is not FR or EFR: the RTP packet being output
is the presumed-bogus speech frame from lower layers, rather than
the intended zero-length payload. Fix this bug.
Related: OS#5975
Change-Id: Icee0f57be289a0592a0197469432a012d15f224c
---
M src/common/l1sap.c
1 file changed, 27 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/28/32128/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32128
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Icee0f57be289a0592a0197469432a012d15f224c
Gerrit-Change-Number: 32128
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: pespin.
falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32128 )
Change subject: rtp continuous-streaming: fix BFI in the quality-suppressed case
......................................................................
Patch Set 1:
(1 comment)
File src/common/l1sap.c:
https://gerrit.osmocom.org/c/osmo-bts/+/32128/comment/21db46c9_17b5bd58
PS1, Line 1661: send_ul_rtp_packet(lchan, fn, bfi, 0);
> bfi is actually not used here (len=0), so better pass NULL to make it clear.
I did not realize that passing a NULL pointer was allowed, for some reason I thought it had to be a pointer to some "real" memory location, like what happens with msg->data when msg->len equals 0. But if you say that NULL is acceptable, then of course it is philosophically more correct - I'll spin a revised patch.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32128
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Icee0f57be289a0592a0197469432a012d15f224c
Gerrit-Change-Number: 32128
Gerrit-PatchSet: 1
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 29 Mar 2023 18:19:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32110 )
Change subject: common+trx: add rtp ecu-downstream vty option
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> So AFAIU you are saying that your ECU is located somewhere in the CN after osmo-msc, as in some sort of PBX/sip-connector.
Correct.
> So, you are actually tweaking the *uplink* RTP path of that BTS here, not the *downlink*. So not sure the "downstream" concept really fits here.
I think I see the source of confusion: the two words "downlink" and "downstream" both begin with the prefix "down", but they are two different English words, and in the present case have completely unrelated meanings. Think of upstream and downstream as in software developers, maintainers and distributors: the original developer's git repository is the ultimate upstream, tarball releases are downstream from that original repo, distro packages are further downstream, and so on. In the present case, I meant "downstream" as in "down the flow of this BTS-originated RTP stream", with the stream flowing through forwarders (OsmoMGW) and translators (themwi-mgw).
> Maybe "rtp uplink-external-eecu" or alike?
I am agnostic on most naming questions, including this one. Harald, you already gave us the name for "rtp continuous-streaming" - maybe you could name this one for us too?
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32110
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I0acca9c6d7da966a623287563e0789db9e0fae8e
Gerrit-Change-Number: 32110
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 29 Mar 2023 18:06:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: falconia <falcon(a)freecalypso.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: falconia.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32128 )
Change subject: rtp continuous-streaming: fix BFI in the quality-suppressed case
......................................................................
Patch Set 1:
(1 comment)
File src/common/l1sap.c:
https://gerrit.osmocom.org/c/osmo-bts/+/32128/comment/a2c3b65a_bc44115a
PS1, Line 1661: send_ul_rtp_packet(lchan, fn, bfi, 0);
bfi is actually not used here (len=0), so better pass NULL to make it clear.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32128
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Icee0f57be289a0592a0197469432a012d15f224c
Gerrit-Change-Number: 32128
Gerrit-PatchSet: 1
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-CC: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Comment-Date: Wed, 29 Mar 2023 18:00:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment