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 OPUS #24

Open
wants to merge 41 commits into
base: master
Choose a base branch
from
Open

Support OPUS #24

wants to merge 41 commits into from

Conversation

FelonEkonom
Copy link
Member

No description provided.

@FelonEkonom FelonEkonom marked this pull request as draft December 12, 2024 17:04
@FelonEkonom FelonEkonom self-assigned this Dec 12, 2024
@FelonEkonom FelonEkonom marked this pull request as ready for review January 17, 2025 10:24
@FelonEkonom FelonEkonom marked this pull request as draft January 20, 2025 09:16
@FelonEkonom FelonEkonom marked this pull request as ready for review January 21, 2025 11:26
@@ -24,7 +24,7 @@ UNIFEX_TERM create(UnifexEnv *env, char *appId, char *token, char *channelId,
scfg.enableVideo = true;
scfg.useStringUid = false;
if (state->service->initialize(scfg) != agora::ERR_OK) {
AG_LOG(ERROR, "Failed to initialize service");
AG_LOG(ERROR, "Failed to initialize sink service");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
AG_LOG(ERROR, "Failed to initialize sink service");
AG_LOG(ERROR, "Failed to initialize Agora service in sink");

Comment on lines 160 to 164
if (codec == CODEC_AUDIO_AAC) {
audioFrameInfo.codec = agora::rtc::AUDIO_CODEC_TYPE::AUDIO_CODEC_AACLC;
} else if (codec == CODEC_AUDIO_OPUS) {
audioFrameInfo.codec = agora::rtc::AUDIO_CODEC_TYPE::AUDIO_CODEC_OPUS;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

[NIT] I would warn if other codec was passed here. I know that now the only options for CodecAudio are CODEC_AUDIO_AAC and CODEC_AUDIO_OPUS, but it might cause trouble in the future if we were to add some further codecs.

state.native_state
)

{[], %{state | native_state: native_state}}
end

@impl true
def handle_stream_format(Pad.ref(:audio, _id), %Opus{}, _ctx, state) do
# audio stream format will be updated in handle_buffer/4
Copy link
Collaborator

Choose a reason for hiding this comment

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

[NIT] this comment is quite ambiguos for me

end

state =
if stream_format == :opus and buffer.metadata.duration != state.last_frame_duration do
Copy link
Collaborator

Choose a reason for hiding this comment

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

metadata.duration isn't available outside of our pipeline, we add it with DurationAddingFilter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

3 participants