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

Support re-use of a single preallocated pcm buffer #28

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

udoprog
Copy link

@udoprog udoprog commented Mar 25, 2021

This adds the following structures and methods:

  • Pcm which is a user provided collection allocated to the size of MAX_SAMPLES_PER_FRAME that dereferences to its stored pcm data &[i16].
  • FrameInfo which just contains the frame header parts of the frame, excluding the data.
  • next_frame_with_pcm which allows for decoding the frame data in-place a previously allocated Pcm structure.
  • next_frame_with_pcm_future async version of next_frame_into.

This allows a caller to preallocate and re-use a single pcm buffer so it doesn't have to be allocated on each call to next_frame to avoid its allocation overhead.

The naming is preliminary, suggestions are welcome!

@germangb
Copy link
Owner

Thanks. I was not a fan of allocating on every single frame either. I'll take a look 😃

Copy link

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just stumbled over this pending PR and left some minor remarks.


Ok(Frame {
data: pcm.data,
sample_rate: frame.sample_rate,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

..frame


Ok(Frame {
data: pcm.data,
sample_rate: frame.sample_rate,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

..frame

/// A MP3 frame, referencing the decoded audio of that frame.
#[derive(Debug, Clone)]
pub struct FrameInfo {
/// This frame's sample rate in hertz.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hertz (Hz)

pub channels: usize,
/// MPEG layer used by this file.
pub layer: usize,
/// Current bitrate as of this frame, in kb/s.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kbit/s? To avoid confusing it with kB or KB.

/// assert_eq!(&pcm[..], &[]);
/// ```
#[derive(Debug)]
pub struct Pcm {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PcmFrame or FrameSamples, consistent with the naming of FrameInfo? Because it is supposed to store the samples decoded from a single MP3 frame.

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.

3 participants