Skip to content

mixer: add logic to maintain data flow when sources are paused #5628

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.

Sign up for GitHub

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

Merged
lgirdwood merged 1 commit into from
Apr 13, 2022
Merged

mixer: add logic to maintain data flow when sources are paused #5628

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 27 additions & 17 deletions src/audio/mixer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ struct mixer_data {
struct ipc4_base_module_cfg base_cfg;
#endif

bool sources_inactive;

void (*mix_func)(struct comp_dev *dev, struct audio_stream *sink,
const struct audio_stream **sources, uint32_t count,
uint32_t frames);
Expand Down Expand Up @@ -343,23 +345,6 @@ static int mixer_copy(struct comp_dev *dev)
return 0;
}

/* write zeros if all sources are inactive */
if (num_mix_sources == 0) {
uint32_t free_frames;
sink = buffer_acquire(sink);
free_frames = audio_stream_get_free_frames(&sink->stream);
frames = MIN(frames, free_frames);
sink_bytes = frames * audio_stream_frame_bytes(&sink->stream);
sink = buffer_release(sink);

if (!audio_stream_set_zero(&sink->stream, sink_bytes)) {
buffer_stream_writeback(sink, sink_bytes);
comp_update_buffer_produce(sink, sink_bytes);
}

return 0;
}

sink = buffer_acquire(sink);

/* check for underruns */
Expand All @@ -372,6 +357,31 @@ static int mixer_copy(struct comp_dev *dev)
sources[i] = buffer_release(sources[i]);
}

if (!num_mix_sources || (frames == 0 && md->sources_inactive)) {
/*
* Generate silence when sources are inactive. When
* sources change to active, additionally keep
* generating silence until at least one of the
* sources start to have data available (frames!=0).
*/
frames = audio_stream_get_free_frames(&sink->stream);

@singalsu singalsu Mar 31, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This fills the amount the sink can fit. Have you checked how much is frames in practice, is it 48 for for 1 ms scheduling? Or does first copy when all sources become inactive push a lot more silence to sink buffer? It might increase buffering delay if some source would become again active. But on this side it's hard to know what is the just sufficient amount to prevent xrun so this is a safe assumption.

@kv2019i kv2019i Mar 31, 2022

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@singalsu This is a good question. The existing code to output silence uses this same logic, i.e. it will as much silence as the sink can fit. I used the same approach here. While running sof-test that trigger this path, it's still usually 48 frames this routine write. So we'd write more if we are really running behind and then it probably makes sense to write more silence to avoid a DAI XRUNs. When the upstream components (that provide data to mixer) are started, there is probably risk of some extra delays in the initial copy() iterations of those components (shouldn't happen but the risk is still there), so definitely not a good time to run low on buffering towards the DAI(s).

@lyakh lyakh Apr 4, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

yes, see my comment above - the MIN() above is bogus, so the normal silence playback does the same

sink_bytes = frames * audio_stream_frame_bytes(&sink->stream);
sink = buffer_release(sink);
if (!audio_stream_set_zero(&sink->stream, sink_bytes)) {
buffer_stream_writeback(sink, sink_bytes);
comp_update_buffer_produce(sink, sink_bytes);
}

md->sources_inactive = true;

return 0;
}
Comment thread
kv2019i marked this conversation as resolved.
Resolved

if (md->sources_inactive) {
md->sources_inactive = false;
comp_dbg(dev, "mixer_copy exit sources_inactive state");
}

sink = buffer_release(sink);

/* Every source has the same format, so calculate bytes based
Expand Down