-
Notifications
You must be signed in to change notification settings - Fork 50
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
Possible inconsistency with RFC 6330: use of ISI vs ESI in PayloadId
#164
Comments
Ah, ya that wouldn't surprise me. I haven't looked at that code in a while.
Want to send a PR? (and a test reproducing the panic would be great!)
…Sent from my phone
On Tue, Mar 5, 2024, 8:22 AM Markus Legner ***@***.***> wrote:
First of all, thank you very much for maintaining this efficient RaptorQ
library, this is highly appreciated. 💯
While integrating this into one of my projects, I believe I found an
inconsistency of the library's behavior with RFC 6330.
RFC 6330, Section 5.3.1
<https://datatracker.ietf.org/doc/html/rfc6330#section-5.3.1> states the
following (emphasis added):
For a given source block of K source symbols, for encoding and decoding
purposes, the source block is augmented with K'-K additional padding
symbols [...]
The encoding symbol ID (ESI) is used by a sender and receiver to identify
the encoding symbols of a source block, [...]. For a source block with K
source symbols, *the ESIs for the source symbols are 0, 1, 2, ..., K-1,
and the ESIs for the repair symbols are K, K+1, K+2, ...*. Using the ESI
for identifying encoding symbols in transport ensures that the ESI values
continue consecutively between the source and repair symbols.
What I get when running the code, however, is a gap between the source and
repair symbols, which should only be there for the internal symbol ID (ISI):
For purposes of encoding and decoding data, the value of K' derived from K
is used as the number of source symbols of the extended source block upon
which encoding and decoding operations are performed, where the K' source
symbols consist of the original K source symbols and an additional K'-K
padding symbols. The Internal Symbol ID (ISI) is used by the encoder and
decoder to identify the symbols associated with the extended source block,
i.e., for generating encoding symbols and for decoding. For a source block
with K original source symbols, the ISIs for the original source symbols
are 0, 1, 2, ..., K-1, the ISIs for the K'-K padding symbols are K, K+1,
K+2, ..., K'-1, and the ISIs for the repair symbols are K', K'+1, K'+2,
....
AFAIU the RFC, the PayloadId should contain the ESI, but the library
actually adds the ISI (in SourceBlockEncoder::repair_packets
<https://github.com/cberner/raptorq/blob/master/src/encoder.rs#L314>).
Using the ESI instead of the ISI would also prevent potential panics in
decode
<https://github.com/cberner/raptorq/blob/master/src/decoder.rs#L264>.
*Example code:*
use raptorq::{Encoder, ObjectTransmissionInformation};
fn main() {
let encoder = Encoder::new(
&[1, 2, 3],
ObjectTransmissionInformation::new(3, 1, 1, 0, 1),
);
for packet in encoder.get_encoded_packets(3) {
println!("{}", packet.payload_id().encoding_symbol_id());
}}
*Example output (K'=10 for K=3):*
0
1
2
10
11
12
—
Reply to this email directly, view it on GitHub
<#164>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGNXQAAZQU4IJT2HOUAFWDYWXWCRAVCNFSM6AAAAABEHOJET2VHI2DSMVQWIX3LMV43ASLTON2WKOZSGE3DSNRTHE2DQMY>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Happy to create a PR for this. This would be a breaking change, however, as the new code would no longer be compatible with the old one. |
That's fine. If it's not currently compliant with the spec we should fix that. I'll just release it as 2.0 to make clear the incompatibility |
I created a PR, please let me know what you think. I didn't add a test for the panic though, as this no longer exists after the changes. |
First of all, thank you very much for maintaining this efficient RaptorQ library, this is highly appreciated. 💯
While integrating this into one of my projects, I believe I found an inconsistency of the library's behavior with RFC 6330.
RFC 6330, Section 5.3.1 states the following (emphasis added):
What I get when running the code, however, is a gap between the source and repair symbols, which should only be there for the internal symbol ID (ISI):
AFAIU the RFC, the
PayloadId
should contain the ESI, but the library actually adds the ISI (inSourceBlockEncoder::repair_packets
). Using the ESI instead of the ISI would also prevent potential panics indecode
.Example code:
Example output (
K'=10
forK=3
):The text was updated successfully, but these errors were encountered: