Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

#64 の修正後、 SoraAudioSink.read() の実行タイミングによってはクラッシュが発生するようになった問題の対応 #66

Merged
merged 2 commits into from
May 28, 2024

Conversation

enm10k
Copy link
Contributor

@enm10k enm10k commented May 21, 2024

変更内容

#64 の修正後に、 SoraAudioSink.read() の実行タイミングによってクラッシュが発生するようになりました

num_of_samples を計算する位置と、 std::condition_variable::wait_for の完了条件を修正して、問題のクラッシュが発生しないようになることを確認しました

再現コード

import faulthandler
import time
import numpy
import sora_sdk

faulthandler.enable()

def on_track(track):
    if track.kind == "audio":
        # 実際に出力することはないので frequery と channels は適当に設定する
        output_frequency = 16000
        output_channels = 1
        audio_sink = sora_sdk.SoraAudioSink(track, output_frequency, output_channels)

        # ここで floating point exception が発生する (Ubuntu)
        # OS によって発生するエラー内容が異なる
        print(audio_sink.read(1, 1))

def test_repro_floating_point_exception():
    SIGNALING_URL = "wss://example.com/signaling"
    CHANNEL_ID = "repro_floating_point_exception"

    sora = sora_sdk.Sora()

    send_conn = sora.create_connection(
        signaling_urls=[SIGNALING_URL],
        role="sendonly",
        channel_id=CHANNEL_ID,
        video=False,
    )

    recv_conn = sora.create_connection(
        signaling_urls=[SIGNALING_URL],
        role="recvonly",
        channel_id=CHANNEL_ID,
        video=False,
    )
    recv_conn.on_track = on_track

    try:
        send_conn.connect()
        recv_conn.connect()

        time.sleep(5)
    finally:
        send_conn.disconnect()
        recv_conn.disconnect()

test_repro_floating_point_exception()

This pull request primarily addresses a bug in the SoraAudioSink.read function in the src/sora_audio_sink.cpp file, as well as updates the CHANGES.md file to reflect these changes. The bug fix ensures that the function does not ignore timeouts and prevents crashes depending on the timing of the read operation.

Bug fix in SoraAudioSink.read:

  • src/sora_audio_sink.cpp: In the SoraAudioSinkImpl::Read function, the calculation of num_of_samples has been moved to after the conditional wait to account for possible updates to number_of_channels_ during the wait. The condition for the wait has also been updated to check that number_of_channels_ is greater than 0 before comparing the buffer size with the number of samples. [1] [2]

Updates to CHANGES.md:

  • CHANGES.md: The fix description for SoraAudioSink.read has been updated to reflect the changes made to address the bug.

@enm10k enm10k requested review from sile and melpon May 21, 2024 02:36
@enm10k enm10k changed the title Feature/fix audio sink read2 #64 の修正後、 SoraAudioSink.read() よってクラッシュが発生する問題の対応 May 21, 2024
@enm10k enm10k changed the title #64 の修正後、 SoraAudioSink.read() よってクラッシュが発生する問題の対応 #64 の修正後、 SoraAudioSink.read() の実行タイミングによってはクラッシュが発生する問題の対応 May 21, 2024
@enm10k enm10k changed the title #64 の修正後、 SoraAudioSink.read() の実行タイミングによってはクラッシュが発生する問題の対応 #64 の修正後、 SoraAudioSink.read() の実行タイミングによってはクラッシュが発生するようになった問題の対応 May 21, 2024
Copy link
Member

@sile sile left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@enm10k enm10k merged commit db50b72 into develop May 28, 2024
3 checks passed
@enm10k enm10k deleted the feature/fix-audio-sink-read2 branch May 28, 2024 01:47
Copy link
Contributor

@melpon melpon left a comment

Choose a reason for hiding this comment

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

よさそう

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants