Error in Decompressor for RTP profile

Bug #604517 reported by jawad Ahmed Saleemi
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
rohc
Status tracked in Rohc-main
1.2.x
Won't Fix
High
Didier Barvaux
1.3.x
Won't Fix
High
Didier Barvaux
Rohc-main
Fix Released
High
Didier Barvaux

Bug Description

Hi,

As discussed over the last email, I am creating this bug tracker for the issue I faces for RTP profile decompression.

1- When using RTP profile, a single erroneous packet desynchronises
 the decompressor and the decompresser is unable to decompress all the
 proceeding packets till the next IR packet arrives.

2- If instead of sending the erroneous packet, it is forced to get lost in the
 channel and never arrives at the decompresser, the decompresser is
 able to decompress all the proceeding packets arriving without any
 problem.

3- While scrutinising, I found that in case of the arrival of the
erroneous packet at the decompresser, the decompresser increments the
"ts_scaled", which should not be incremented until a packet with
correct CRC arrives , as per the standards. This leads to the wrong
calculated CRC values (in do_decode_uo0_and_uo1()) for the next
arriving packets.

4- When using only IP/UDP profile, this problem does not appear, and
the decompresser is able to decompress the packets proceeding
erroneous packet since "ts_scaled" is not involved in this case.

All these facts lead to the conclusion that there is something wrong
at the decompresser side with RTP profile or more specifically with
"ts_sc_decomp.c".
For UO0-*, UO1-* and UOR-2-* packets, the decompression
context is updated with values deduced from a ROHC packet that fails CRC
check. The context should not be updated. For IR and IR-DYN packets, it
seems that the CRC is checked before any context update, so IR and
IR-DYN packets should be bug-free.

I would really appreciate if you can fix the bug and send me the updated patch.
Thanks,
regards,
Jawad

Revision history for this message
jawad Ahmed Saleemi (jawad-saleemi) wrote :
Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

Versions 1.2.x are affected.

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

Versions 1.3.x are affected.

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

Trunk is also affected.

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

The bug will not be fixed in the 1.2.x branch. It would require a large rework of the code base. The README file was updated to mention the existing bug. See http://bazaar.launchpad.net/~didier-barvaux/rohc/1.2.x/revision/128

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

The bug will not be fixed in the 1.3.x branch. It would require a large rework of the code base. The README file was updated to mention the existing bug. See http://bazaar.launchpad.net/~didier-barvaux/rohc/1.3.x/revision/158

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

I started working on this problem on the trunk.

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

I have just added a new robustness test that checks that the library correctly handle damaged packets. The test voluntary damages a ROHC packet before decompressing it. Decompression of the damaged packet shall fail but decompression of next packets shall succeed. See http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/239

There is only one input file (damage done on UO-0 packet) currently and the test fails with the current codebase in trunk.

Next step #1: add more input files (IR with TSS bit set, IR with TSS bit not set, IR-DYN with TSS bit set, IR-DYN with TSS bit not set, UO-1, UOR-2 without extension 3, UOR-2 with TS_STRIDE updated in extension 3, UOR-2 with TS_STRIDE not updated in extension 3).

Next step #2: change the trunk code to make all the tests succeed (work already in progress).

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

I have just added 2 additional input files:
 - IR with TSS bit set,
 - IR with TSS bit not set.

See http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/242

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

I have just added 2 additional input files:
 - IR-DYN with TSS bit set,
 - IR-DYN with TSS bit not set.

See http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/243

tags: added: conformance
Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

I have just enhanced the test to check that the damaged ROHC packet is of the expected type (IR, IR-DYN, UO-0, UO-1, UOR-2).
See http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/244

I have also added 1 additional input file: UO-1.
See http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/245

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

The following test cases remain to be setup:
 - UOR-2 without extension 3,
 - UOR-2 with TS_STRIDE updated in extension 3,
 - UOR-2 with TS_STRIDE not updated in extension 3.

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

Add new input files for the robustness test against damaged packets:
 - damage an UOR-2 packet without extension 3,
 - damage an UOR-2 packet with extension 3 and TS_STRIDE optional field present.
 - damage an UOR-2 packet with extension 3 and TS_STRIDE optional field not present.

See http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/246 and http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/252

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

After several commits adding the robustness tests, here is the bugfix:
   - When decoding new TS_STRIDE, TS_SCALED and TS_OFFSET values, do not replace
     their old values immediately. Wait for the packet's CRC to be successfully
     checked before doing so. This way, damaged packets do not update the
     decompression context resulting in subsequent decompression failures.
   - Do not send the TS_SCALED bits too early. Stay in the INIT_STRIDE state
     until at least some packets with TS_STRIDE bits are transmitted. This is
     required to be confident that the TS_STRIDE value was successfully
     transmitted to the decompressor. The default value was set to 3 packets.
   - All added tests for that bug now work fine. This is a good thing :-)

See http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/253

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

The bug is now fixed in the trunk.

tags: added: library robustness
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.