Skip to content

MAVLinkX Decode Methods#410

Open
jlpoltrack wants to merge 5 commits intoolliw42:mainfrom
jlpoltrack:encode-decode-lut
Open

MAVLinkX Decode Methods#410
jlpoltrack wants to merge 5 commits intoolliw42:mainfrom
jlpoltrack:encode-decode-lut

Conversation

@jlpoltrack
Copy link
Copy Markdown
Collaborator

Use LUT for MAVLinkX Encode/Decode

@olliw42
Copy link
Copy Markdown
Owner

olliw42 commented Feb 14, 2026

have you critically tested this?
I'm not convinced this is right code.

first off, I think the "big" encode LUT is a bit of overdoing it, and it is actuially not so clear that it really brings a gain. Unless prooven with hard facts worth it I would not do that.

the "small" decode LUT is something I have given a couple of serious attempts, but I never could get it worked out how to do the final byte, i.e., termination of the stream.
In this code I do not see anything which would address that. To me it looks too simple, and I in fact think line 1191
if (fmavx_status.in_pos >= len) goto decompress_done; // not enough bits, EOF
is simply incorrect. I might be worng and just fail to see the magic, but I'm not convinced this is correct code.

@jlpoltrack
Copy link
Copy Markdown
Collaborator Author

Before I spend time on - is it worth measuring how long the current decode takes to see if this is worth trying to improve? For example, if it takes 50 uS and can only happen 19 / sec then probably not worth the efforts.

@olliw42
Copy link
Copy Markdown
Owner

olliw42 commented Feb 14, 2026

I think it would be interesting to measure the consumed time alone from a general perspective, to just know that better.

The "small" decode LUT is like the part which has the largest impact, since it potentially has to read multiple times in tiny bit junks. So, while I agree that it might not be most important, I personally just have a sort of eagerness ... it naggs me :)

One "argument" for getting it fast enough could be, that if it's fast enough we could also enable compression for, e.g., teh 31 Hz mode. 50Hz and higher makes probably no sense, but 31 Hz is just sufficiently enough, so here it could make sense.

As regards my concerns, let me give a case which I think which currently fails:
It is well possible that the last two bits in the last byte is a valid symbol. However, if one always reads in 5 bits, and errors out if one can't read all 5, that symbol is obviosuly missed.
This is a complexity of the compression protocol used here, that it doesn't have a separate EOF symbol. I felt forced to avoid it since it actually would have reduced the efficiency of the compression, and logically it is not needed.

@olliw42
Copy link
Copy Markdown
Owner

olliw42 commented Feb 14, 2026

the fix looks AI generated but teh AI not really seeing the issue
you can make the argument also for 3 bits and 4 bits :) it was to just give an example of one case

I guess a working approach could be to do the LUT until the third last byte, and then switch to the bits approach for the rest ... but isn't nice code at all ... I always thought there must be a smarter way.

Anyway, the point is, there is some complexity in here.

@jlpoltrack jlpoltrack changed the title LUT for Encode/Decode MAVLinkX Decode Methods Feb 18, 2026
@jlpoltrack
Copy link
Copy Markdown
Collaborator Author

Had the tool propose a few different decode methods (all over my head).

I measured relative performance using TS_START / TS_END and saw:

  • Fused is the fastest
  • Split / Peek generally 2nd
  • LUT behind
  • Original behind

On 48 MHz CPU - I saw around 1 ms per second spent decoding with the original method and ~775 uS with fused. Others fall in between this.

@olliw42
Copy link
Copy Markdown
Owner

olliw42 commented Feb 18, 2026

a quick look didn't allow mne to understand the different techniques, will have to look at it sometimes else

just to get the reference right: "original" refers to what we currently use in v1.3.08, i.e., with the bit buffers, or does it refer to what we had all time before?

@jlpoltrack
Copy link
Copy Markdown
Collaborator Author

just to get the reference right: "original" refers to what we currently use in v1.3.08, i.e., with the bit buffers, or does it refer to what we had all time before?

Correct

@olliw42
Copy link
Copy Markdown
Owner

olliw42 commented Feb 18, 2026

ok, I'm then not surprised that the further efforst don't bring that much. I think I may have mentioned that IMHO the bit buffers should have brought the biggest improvement. It's fantastic though to have some numbers.

when you say ~ 1 ms, then this refers to what?
A cycle, i.e., 52 ms in the 19 Hz mode, or a 1 sec? I've not used TS_xxxx do far :knock_head:. I guess it's 1 sec, right.
If so, ~ 1ms would be about 55 us per cycle ... that's pretty affordable, given it's also only 48 Hz ... if that's representative we may think enabling it for 31 Hz too (after v1.4)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants