-
-
Notifications
You must be signed in to change notification settings - Fork 159
support for raw frames in PacketEncoder #691
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
base: main
Are you sure you want to change the base?
Conversation
in my opinion the fields should be public because the structure is low level
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really understand your use case for this that well. Can you elaborate some more on you situation?
self.buf.put_bytes(0, data_prefix_len); | ||
self.buf | ||
.copy_within(start_len..start_len + data_len, start_len + data_prefix_len); | ||
.copy_within(from..from + data_len, from + data_prefix_len); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I don't like the arbitrary var name changes. IMO this makes the code less readable, and the extra diff makes harder to review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, maybe i was too lazy with this, this is looks like this bc i didnt interpreted the code chunk that compresses the packet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think the new names for the vars make sense, because "start_len" was used for the length of the buffer before pushing the frame's bytes, but now it makes sense that its named "from", i changed the var name because i felt that "start_len" didnt maked that much sense for the argument's var name
/// frames the bytes in a range from `from` to the end of the buffer, framing is: | ||
/// adding a packet length varint to the start of the frame | ||
/// adding a data length varint after the packet length, if compression is enabled | ||
/// compressing the packet, if compression is enabled | ||
pub fn enframe_from(&mut self, from: usize) -> anyhow::Result<()> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "Framing" is not common terminology. It's either obfuscating a concept that can be described in more simple terms, or its adding just adding jargon to something that's already complex.
i want to be able to push raw stuff like |
Objective
its not possible to push a frame to the
PacketEncoder
if its notPacket + Encode
, this means that if i have a raw frame i can't push it because its not deserialized, so i cant usePacketEncoder
to compress/encrypt my raw packetsSolution
i added
enframe_from
method, which enframes a rightmost portion of the buffer which includes adding a packet length varint, a data length varint and compressing, this was already done internally by theappend_packet
method but now its a method so we can re-use this logic, and of courseappend_frame
andprepend_frame
methods