Systematic FNT #239
Systematic FNT #239
Conversation
|
amazing ! |
lamphamsy
force-pushed
the
ft/systematic
branch
from
127c10d to
ae62a24
Compare
lamphamsy
force-pushed
the
ft/systematic
branch
from
3a7ec57 to
8f3fb3b
Compare
lamphamsy
force-pushed
the
eh/decoding
branch
from
7206545 to
85433c3
Compare
There was a problem hiding this comment.
We introduced a lot of if (type == FecType::SYSTEMATIC) in the code, maybe it would be worth it to make the FEC type a template parameter (you're unlikely to change the code type at runtime, after instantiating the object anyway), that would allow to specialize the code (less branch, more efficient/readable code).
To be investigated in another PR maybe.
src/fft_naive.h
Outdated
| { | ||
| fft_inv(output, input); | ||
|
|
||
| /* |
There was a problem hiding this comment.
Can be replaced by a single line comment: // We need to divide output to N for the inverse formular.
src/fft_2n.h
Outdated
| @@ -328,7 +329,13 @@ void Radix2<T>::fft_inv(vec::Buffers<T>& output, vec::Buffers<T>& input) | |||
| } | |||
|
|
|||
| // copy input to output | |||
There was a problem hiding this comment.
This comment can be removed, seeing that you're using the method copy, I think it's obvious.
|
|
||
| if (type == FecType::SYSTEMATIC) { | ||
| this->fft->fft(*dec_inter_codeword, output); | ||
| for (unsigned i = 0; i < this->n_data; i++) { |
There was a problem hiding this comment.
We really should use size_t here, especially on 64-bit.
I think after this PR you need to tackle #243 next.
src/fec_rs_fnt.h
Outdated
| private: | ||
| // buffers for intermediate symbols used for systematic FNT | ||
| std::unique_ptr<vec::Buffers<T>> inter_words; | ||
| // buffers for suffix symbols of cocdewords used for systematic FNT |
There was a problem hiding this comment.
What is a cocdewords? Did you mean codewords or codecwords ?
There was a problem hiding this comment.
Thanks, it should be codewords :)
test/ec_driver.cpp
Outdated
| @@ -171,7 +173,10 @@ bool repair_data_files(quadiron::fec::FecCode<T>* fec) | |||
| } | |||
| } | |||
|
|
|||
| fec->decode_bufs(d_files, c_files, c_props, r_files); | |||
| if (operation_on_packet) | |||
There was a problem hiding this comment.
Use curly braces (I haven't enable the linter for this, but it will come one day/soon).
if (operation_on_packet) {
fec->decode_packet(d_files, c_files, c_props, r_files);
} else {
fec->decode_bufs(d_files, c_files, c_props, r_files);
}| @@ -50,7 +50,7 @@ void RsFnt<uint16_t>::encode_post_process( | |||
| { | |||
There was a problem hiding this comment.
This commit should either have a proper commit message, or be squashed with the commit that introduced the bug.
Commit without proper commit message are just polluting the git log and makes git blame less useful to understand a code base.
There was a problem hiding this comment.
It will be squashed
There was a problem hiding this comment.
I will rewrite the commit message :)
src/fec_rs_fnt.h
Outdated
|
|
||
| if (this->type == FecType::SYSTEMATIC) { | ||
| // for encoding | ||
| enc_frag_ids = std::unique_ptr<vec::Vector<T>>( |
There was a problem hiding this comment.
If my PR to bump to C++14 is merged, you can use make_unique instead
lamphamsy
force-pushed
the
eh/decoding
branch
3 times, most recently
from
79f6c83 to
e0a618e
Compare
lamphamsy
force-pushed
the
ft/systematic
branch
3 times, most recently
from
99b94e1 to
1ff011b
Compare
Output length should be compatible for both of systematic and non-systematic FNT
lamphamsy
force-pushed
the
ft/systematic
branch
from
1ff011b to
c847089
Compare
|
I've just rebased + squashed commits |
Add encode/decode for systematic FNT codes