https://osmocom.org/https://osmocom.org/favicon.ico?16647414092022-02-23T12:13:52ZOpen Source Mobile CommunicationsOsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=236942022-02-23T12:13:52Zneelsnhofmeyr@sysmocom.de
<ul></ul><p>Sounds good to me, thanks for the explanation, Pau!</p> OsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=237352022-03-03T14:15:21Zdexter
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>50</i></li></ul><p>To ensure that we do not run again in the bwe<->oa conversion problem we have seen recently I have increased the test coverage in the TTCN3 tests. When I revert my previous patch, then TC_amr_oa_bwe_rtp_conversion fails, which shows that it works.</p>
<p><a class="external" href="https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/27384">https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/27384</a> MGCP_Test: ensure PT translation works when converting AMR bwe/oa</p>
<p>I have now changed the mgcp_codec_pt_translate() and the related unit tests. The behavior is only slightly changed, but I don't think that this is of any practical relevance.</p>
<p><a class="external" href="https://gerrit.osmocom.org/c/osmo-mgw/+/27391">https://gerrit.osmocom.org/c/osmo-mgw/+/27391</a> Revert "mgcp_codec: do not differentiate between oa and bwe when comparing ...<br /><a class="external" href="https://gerrit.osmocom.org/c/osmo-mgw/+/27392">https://gerrit.osmocom.org/c/osmo-mgw/+/27392</a> mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate()</p>
<p>We probably might investigate though what really happens when one side has for example octet-aligned and the other side has both payload formats assigned. I doubt that this will work with the current code. For that kind of case we will need a TTCN3 test and probably make the choice of the egress codec a little more dynamic. Apart from that I think this fix is restoring the intended behavior as it was.</p>
<p>When I am understanding the issue correctly, then the concern was that a PBX on the SIP side may assign two AMR modes with two different PT numbers for compatibility reasons.</p> OsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=237362022-03-03T14:21:15Zdexter
<ul></ul><p>So a concrete scenario would be:</p>
<p>On the osmo-cn side use e.g. PT 112 for AMR in oa.</p>
<p>On the SIP side we use PT 113 for AMR in oa and PT 114 for AMR in bwe.</p>
<p>==> The MGW will notice that 112<->113 is requiring no format conversion, it will send out the packets to the SIP side under PT 113, while sending no packets under PT 114 at all. Any bwe packet that it receives under PT 114 will be converted to oe and forwarded to the osmo-cn side under PT 112. Any packet that osmo-mgw receives under PT 113 will just forwarded unchanged to the osmo-cn side under PT 112 as it is already oa formatted.</p> OsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=237372022-03-03T14:35:19Zpespin
<ul></ul><p>dexter wrote:</p>
<blockquote>
<p>When I am understanding the issue correctly, then the concern was that a PBX on the SIP side may assign two AMR modes with two different PT numbers for compatibility reasons.</p>
</blockquote>
<p>Yes correct, that's the scenario I had in mind.</p>
<blockquote>
<p>So a concrete scenario would be:</p>
<p>On the osmo-cn side use e.g. PT 112 for AMR in oa.</p>
</blockquote>
<p>You probably mean the osmo-bss side here, since the one handling SIP is the MSC (which is the CN).</p>
<blockquote>
<p>On the SIP side we use PT 113 for AMR in oa and PT 114 for AMR in bwe.</p>
<p>==> The MGW will notice that 112<->113 is requiring no format conversion, it will send out the packets to the SIP side under PT 113, while sending no packets under PT 114 at all. Any bwe packet that it receives under PT 114 will be converted to oe and forwarded to the osmo-cn side under PT 112. Any packet that osmo-mgw receives under PT 113 will just forwarded unchanged to the osmo-cn side under PT 112 as it is already oa formatted.</p>
</blockquote>
<p>Yes, that's the ideal solution IMHO.</p>
<p>I think it should be failrly easy to write a TTCN3 test validating that exact scenario, and then we make sure we have no regressions there, which may be difficult to spot.</p> OsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=264522023-03-22T16:09:54Zdexter
<ul></ul><p>I have rebased the two pending patches:</p>
<p><a class="external" href="https://gerrit.osmocom.org/c/osmo-mgw/+/27391">https://gerrit.osmocom.org/c/osmo-mgw/+/27391</a> Revert "mgcp_codec: do not differentiate between oa and bwe when comparing ...<br /><a class="external" href="https://gerrit.osmocom.org/c/osmo-mgw/+/27392">https://gerrit.osmocom.org/c/osmo-mgw/+/27392</a> mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate()</p>
<p>Both patches must be merged at the same time since the first one is just a revert to a previous patch.</p>
<p>The next task here would be to create a TTCN3 test that simulates the behavior discussed above.</p> OsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=264982023-03-27T14:00:45Zdexter
<ul></ul><p>I have now fixed the review issues and also refactored the code mgcp_codec.c a bit, see patch: <a class="external" href="https://gerrit.osmocom.org/c/osmo-mgw/+/32083">https://gerrit.osmocom.org/c/osmo-mgw/+/32083</a> mgcp_codec: refactor payload type converstion</p> OsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=265422023-03-29T16:02:31Zdexter
<ul></ul><p>I am almost done with extending the RTP Emulation and the MGCP_Test so that it supports multiple codecs with multiple pairs of RX/TX octet strings. Some bits are missing but once it is done we can verify if the octet-align/banwith-efficient conversation works as intended when both modes are allowed on one side.</p> OsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=265622023-03-30T12:16:28Zdexter
<ul></ul><p>The patch that extends the RTP-Emulation and the MCGP/SDP generation is now up for review: <a class="external" href="https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32153">https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32153</a></p> OsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=266702023-04-05T12:34:28Zdexter
<ul><li><strong>% Done</strong> changed from <i>50</i> to <i>80</i></li></ul><p>I have now added a few more testcases that specifically address the problem when AMR is assigned twice with octet_aligned=0 and octet_aligned=1:</p>
<p><a class="external" href="https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32220">https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32220</a> MGCP_Test: add new testcases for oa/bwe conversion</p>
<p>The added tests showed that there are problems with the codec decision. This is now also fixed: <a class="external" href="https://gerrit.osmocom.org/c/osmo-mgw/+/32218">https://gerrit.osmocom.org/c/osmo-mgw/+/32218</a></p> OsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=267672023-04-25T13:04:39Zdexter
<ul></ul><p>The patchset for osmo-mgw seems to have no open issues, but still requires some more review:<br /><a class="external" href="https://gerrit.osmocom.org/c/osmo-mgw/+/32218">https://gerrit.osmocom.org/c/osmo-mgw/+/32218</a><br /><a class="external" href="https://gerrit.osmocom.org/c/osmo-mgw/+/32290">https://gerrit.osmocom.org/c/osmo-mgw/+/32290</a></p>
<p>The corresponding patchset for the MGCP TTCN3 tests is almost done. There is only one patch that does not have enough review points yet:<br /><a class="external" href="https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32153">https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32153</a><br />(as far as I can see everything is ok here)</p> OsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=268142023-05-02T14:42:57Zdexter
<ul><li><strong>% Done</strong> changed from <i>80</i> to <i>90</i></li></ul><p>The following patches are currently still in review:</p>
<p><a class="external" href="https://gerrit.osmocom.org/c/osmo-mgw/+/32506">https://gerrit.osmocom.org/c/osmo-mgw/+/32506</a> mgcp_codec: move mgcp_codec_decide down<br /><a class="external" href="https://gerrit.osmocom.org/c/osmo-mgw/+/32218">https://gerrit.osmocom.org/c/osmo-mgw/+/32218</a> mgcp_codec: fix codec decision<br /><a class="external" href="https://gerrit.osmocom.org/c/osmo-mgw/+/32290">https://gerrit.osmocom.org/c/osmo-mgw/+/32290</a> mgcp_network: do not deliver RTP packets with unpatched PT</p>
<p>I noticed that the related TTCN3 patch is ready to submit but I think its better to wait until the mgw patches are ready as well.</p> OsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=268312023-05-04T10:37:34Zdexter
<ul></ul><p>I have now merged the part of the TTCN3 patchset that introduces support for multiple RTP payloads since those patches do not change the testsuite itself. This probably makes more sense so anyone can use the multiple RTP payload feature now and we also may avoid merge conflicts.</p>
<p>The following patch is not yet merged since it adds testcase specific to the codec decision fixes in osmo-mgw (see above), which do not have enough review points yet.<br /><a class="external" href="https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32220">https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32220</a> MGCP_Test: add new testcases for oa/bwe conversion</p> OsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=269432023-05-22T09:30:34Zdexter
<ul></ul><p>The following patch had review issues, which are fixed by now. It is also the only patch in this patch set that is not ready yet:<br /><a class="external" href="https://gerrit.osmocom.org/c/osmo-mgw/+/32218">https://gerrit.osmocom.org/c/osmo-mgw/+/32218</a> mgcp_codec: fix codec decision</p> OsmoMGW - Bug #5461: Fix regression translating AMR Payload Type between connectionshttps://osmocom.org/issues/5461?journal_id=270072023-05-31T09:55:40Zdexter
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li><li><strong>% Done</strong> changed from <i>90</i> to <i>100</i></li></ul><p>This can now be resolved. Unfortunately the patchset caused problems with MGCP_Test.TC_two_crcx_mdcx_and_iuup_rtp and MGCP_Test.TC_two_crcx_mdcx_and_iuup_rtp_rfci_unordered because IuFP was not taken into account. This is now fixed and the two affected TTCN3 tests pass again.</p>