Skip to content
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

Is possible that decoder support PacketRef? #166

Open
simon-fu opened this issue Dec 4, 2022 · 4 comments
Open

Is possible that decoder support PacketRef? #166

simon-fu opened this issue Dec 4, 2022 · 4 comments
Labels
enhancement New feature or request
Milestone

Comments

@simon-fu
Copy link

simon-fu commented Dec 4, 2022

Suppose application has a fixed buffer to store every packet data, but before invoking Decoder::decode() it must invoke Packet::new() to alloc data first. That make poor performance. Is possible that add a new type PacketRef and change decode function signature to Decoder::decode(&PacketRef) ?
The PacketRef look like:
pub PacketRef<'a> { .. the same as Packet, data: &'a [u8], }

@pdeljanov
Copy link
Owner

Hey @simon-fu,

I've run into the same problem too when trying to do a zero-copy decode. We could likely improve this in v0.6 since we would be making breaking changes to the API.

That being said, have you found that to be a major performance problem? Packets are generally small so the allocation should be pretty fast.


My previous ideas:

  1. Packet could become an enum of either a borrowed slice or owned Box, but that might introduce tricky lifetime problems.
  2. Have a second decode function that takes a &[u8] slice or PacketRef as you described.

@pdeljanov pdeljanov added the enhancement New feature or request label Dec 7, 2022
@pdeljanov pdeljanov added this to the v0.6.0 milestone Dec 7, 2022
@fengalin
Copy link

fengalin commented Dec 7, 2022

Just wanted to note that this seems related: #109

@pdeljanov
Copy link
Owner

Just wanted to note that this seems related: #109

Yep, a similar concern, but just at the output side.

Thanks for your patience regarding that PR. I'm fairly confident we can address both issues in v0.6, but I just want to get a bunch of quality and performance fixes out in v0.5.2 first.

I have many new thoughts to share on the relationship between AudioBuffers and SampleBuffers that hopefully we can use to further improve the API.

@simon-fu
Copy link
Author

Hey @simon-fu,

I've run into the same problem too when trying to do a zero-copy decode. We could likely improve this in v0.6 since we would be making breaking changes to the API.

That being said, have you found that to be a major performance problem? Packets are generally small so the allocation should be pretty fast.

My previous ideas:

  1. Packet could become an enum of either a borrowed slice or owned Box, but that might introduce tricky lifetime problems.
  2. Have a second decode function that takes a &[u8] slice or PacketRef as you described.

Thanks for reply.
I have no major performance problem. I just think that allocation on heap is expensive and zero-copy is Rustaceans' goal :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants