Skip to content

Conversation

@xianshijing-lk
Copy link
Collaborator

This implement the sdl_media_manager, sdl_media, sdl_video_renderer that can do the audio / video capturing and rendering.

And this PR also hooks it up with our SDK

Copy link

@cloudwebrtc cloudwebrtc left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link

@ladvoc ladvoc left a comment

Choose a reason for hiding this comment

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

LGTM ✅

RoomInfoData room_info() const;

// Get the local participant.
//
Copy link

Choose a reason for hiding this comment

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

Should these use /// so they can be picked up by the documentation generator later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, fix now.

static_cast<uint64_t>(track->ffi_handle_id()));
new_audio_stream->set_sample_rate(options_.sample_rate);
new_audio_stream->set_num_channels(options.num_channels);
// TODO, sample_rate and num_channels are not useful in AudioStream, remove it
Copy link

Choose a reason for hiding this comment

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

Could you please elaborate on what you mean by this? If this is the case, we can mark the fields as deprecated in the FFI Protobuf definition.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is what I found when implementing the AudioStream:
In the Rust implementation, AudioStream acts as a container that gathers audio buffers from the underlying audio track. Each audio buffer provided by the track already has its own sample_rate and num_channels.

The AudioStream simply exposes these buffers through its read() function, it does not resample the audio buffer to respect or enforce the sample_rate or num_channels specified when initializing the AudioStream.

That said, I think it’s correct that AudioStream does not include a built-in resampler, since that would introduce unnecessary performance overhead. However, in that case, it shouldn’t accept sample_rate or num_channels as initialization parameters.

@xianshijing-lk xianshijing-lk merged commit 1319391 into main Dec 2, 2025
5 checks passed
@xianshijing-lk xianshijing-lk deleted the sxian/CLT-2315/implement_sdl_media_manager_hookup_audio_video_streams branch December 2, 2025 18:43
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.

4 participants