Streams-R-Us/atem-connection-rs
3
Fork 0
Code Issues Pull Requests 1 Packages Projects Releases Wiki Activity

WIP: towards no_std, zerocopy, field parsing #1

Draft
samw wants to merge 10 commits from somethings into main
pull from: somethings
merge into: Streams-R-Us:main
Conversation 1 Commits 10 Files Changed 37 +1263 -283
samw commented 2025-06-12 18:35:51 +01:00
Owner
Copy Link

A collection of changes aimed at no-std support and efficient decoding of just what the client is interested in. Maybe.

  • Make AtemPacket zero copy
    • .. tho given the body was zero copy in the tally branch this is less impactful than when I started on old main - maybe this fully-zero-copy isn't necessary. But then, bit shifts are very cheap...
    • Add lazy field (what the rest of the project calls command, but I've kept separate since i think it's worth having a distinction between RX (fields) and TX (commands)) parsing as an iterator (raw_fields) and a somewhat experimental rich parsing based on a Field trait. See below.
  • Update to edition 2024
    • Replace macro_use with explicit imports in each file
    • Unsafe now required around env calls
  • Make no_std
    • remove std stuff from atem_packet.rs
    • add feature
    • make no_std, with extern crate std behind a feature
      • This means the std prelude is not implicit, so we have to use all the std stuff

Field parsing

I'm not too sure about this - I was approaching it from the direction of "make parsing for just the fields you need as cheap as possible" with an eye for embedded no_std tally devices which only care about a few of the possible field types. But I don't know how nice this is in practice given with my current .fields<F>() iterator you'd need to iterate over the packet's fields N times for every N types of field you were interested in, which seems a bit sad.

A collection of changes aimed at no-std support and efficient decoding of just what the client is interested in. Maybe. - Make AtemPacket zero copy - .. tho given the body was zero copy in the `tally` branch this is less impactful than when I started on old `main` - maybe this fully-zero-copy isn't necessary. But then, bit shifts are very cheap... - Add lazy field (what the rest of the project calls command, but I've kept separate since i think it's worth having a distinction between RX (fields) and TX (commands)) parsing as an iterator (`raw_fields`) and a somewhat experimental rich parsing based on a `Field` trait. See below. - Update to edition 2024 - Replace `macro_use` with explicit imports in each file - Unsafe now required around env calls - Make `no_std` - remove std stuff from atem_packet.rs - add feature - make no_std, with extern crate std behind a feature - This means the std prelude is not implicit, so we have to `use` all the std stuff ## Field parsing I'm not too sure about this - I was approaching it from the direction of "make parsing for just the fields you need as cheap as possible" with an eye for embedded no_std tally devices which only care about a few of the possible field types. But I don't know how nice this is in practice given with my current `.fields<F>()` iterator you'd need to iterate over the packet's fields N times for every N types of field you were interested in, which seems a bit sad.
samw added 4 commits 2025-06-12 18:35:52 +01:00
samw force-pushed somethings from 2ec4c231c2 to cd614c0956 2025-06-19 10:08:27 +01:00 Compare
sbaudlr reviewed 2025-06-21 15:48:19 +01:00
sbaudlr left a comment
Owner
Copy Link

Just doing a high-level read of things and leaving thoughts

Just doing a high-level read of things and leaving thoughts
atem-connection-rs/src/atem_lib/atem_field.rs
@ -0,0 +2,4 @@
/// An uninterpreted ATEM protocol field - a 4-ascii character type plus variable length data
pub struct RawField<'a> {
pub r#type: &'a str,
sbaudlr commented 2025-06-21 15:14:33 +01:00
Owner
Copy Link

nitpick: I don't think r#type is super clear, could be raw_type or similar?

nitpick: I don't think `r#type` is super clear, could be `raw_type` or similar?
atem-connection-rs/src/atem_lib/atem_field.rs
@ -0,0 +26,4 @@
#[derive(Debug)]
pub struct PrvI {
pub m_e_index: u8,
sbaudlr commented 2025-06-21 15:23:44 +01:00
Owner
Copy Link

nitpick: Super a personal preference thing but I prefer not to contract names and instead write e.g. mix_effect_index for clarity.

nitpick: Super a personal preference thing but I prefer not to contract names and instead write e.g. `mix_effect_index` for clarity.
atem-connection-rs/src/atem_lib/atem_field.rs
@ -0,0 +60,4 @@
}
}
pub trait Field<'a>: Sized {
sbaudlr commented 2025-06-21 15:15:58 +01:00
Owner
Copy Link

suggestion: File clarity could be improved by moving this trait definition up to before any of the implementations of this trait.

suggestion: File clarity could be improved by moving this trait definition up to before any of the implementations of this trait.
atem-connection-rs/src/atem_lib/atem_field.rs
@ -0,0 +61,4 @@
}
pub trait Field<'a>: Sized {
const TYPE: &'static str;
sbaudlr commented 2025-06-21 15:21:56 +01:00
Owner
Copy Link

nitpick: I don't think TYPE is clear, as to me this suggests "integer" or "string" or similar, whereas it's more to do with the name/label of the field

nitpick: I don't think `TYPE` is clear, as to me this suggests "integer" or "string" or similar, whereas it's more to do with the name/label of the field
atem-connection-rs/src/atem_lib/atem_packet.rs Outdated
@ -60,0 +116,4 @@
///
/// Returns None if this is a packet without fields.
pub fn raw_fields(&self) -> RawFields {
// TODO: do we only ever get newsessionid during the handshake (i.e. not in a packet with fields)?
sbaudlr commented 2025-06-21 15:34:13 +01:00
Owner
Copy Link

iirc, yes.... in normal operation. If we see something from the ATEM we don't like (e.g. it sending us a handshake acknowledgement out-of-sequence) or if our connection to the ATEM times out then we should destroy our held state about the connection and recreate it and gain a new session Id (which is what is implemented on main). So yeah, outside of the handshake we don't need to handle a new session (I believe)

iirc, yes.... in normal operation. If we see something from the ATEM we don't like (e.g. it sending us a handshake acknowledgement out-of-sequence) or if our connection to the ATEM times out then we should destroy our held state about the connection and recreate it and gain a new session Id (which is what is implemented on main). So yeah, outside of the handshake we don't need to handle a new session (I believe)
atem-connection-rs/src/atem_lib/atem_packet.rs Outdated
@ -115,0 +170,4 @@
if remain == 0 {
return None;
} else if remain < 8 {
// TODO: is 8 indeed the minimum size for something here? (i.e. no field data)
sbaudlr commented 2025-06-21 15:41:59 +01:00
Owner
Copy Link

12 bytes is the minimum packet length, if a packet is more than 12 bytes then the field data will always be aligned to 8 bytes iirc

12 bytes is the minimum packet length, if a packet is more than 12 bytes then the field data will always be aligned to 8 bytes iirc
samw added 3 commits 2025-06-21 15:58:24 +01:00
This pull request is marked as a work in progress.
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin somethings:somethings
git checkout somethings
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
q samw sbaudlr
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Streams-R-Us/atem-connection-rs#1
No description provided.
Delete Branch "somethings"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?