From 01181e6e22e5cfc9d0cb17231f2c1866cc53b04a Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Sat, 13 Aug 2016 16:29:18 +0200 Subject: [PATCH] Fix a deadlock issue when shutting down ALSA cards. --- alsa_input.cpp | 6 +++++- alsa_input.h | 2 +- audio_mixer.cpp | 32 ++++++++++++++++++++------------ audio_mixer.h | 13 +++++++++---- mixer.cpp | 5 ++++- 5 files changed, 39 insertions(+), 19 deletions(-) diff --git a/alsa_input.cpp b/alsa_input.cpp index 22daf76..bfadacb 100644 --- a/alsa_input.cpp +++ b/alsa_input.cpp @@ -125,7 +125,11 @@ void ALSAInput::capture_thread_func() const int64_t prev_pts = frames_to_pts(num_frames_output); const int64_t pts = frames_to_pts(num_frames_output + frames); - audio_callback(buffer.get(), frames, audio_format, pts - prev_pts); + bool success; + do { + if (should_quit) return; + success = audio_callback(buffer.get(), frames, audio_format, pts - prev_pts); + } while (!success); num_frames_output += frames; } } diff --git a/alsa_input.h b/alsa_input.h index d485a3b..895760f 100644 --- a/alsa_input.h +++ b/alsa_input.h @@ -22,7 +22,7 @@ class ALSAInput { public: - typedef std::function audio_callback_t; + typedef std::function audio_callback_t; ALSAInput(const char *device, unsigned sample_rate, unsigned num_channels, audio_callback_t audio_callback); ~ALSAInput(); diff --git a/audio_mixer.cpp b/audio_mixer.cpp index acd48c5..1c570fa 100644 --- a/audio_mixer.cpp +++ b/audio_mixer.cpp @@ -123,7 +123,7 @@ AudioMixer::~AudioMixer() void AudioMixer::reset_resampler(DeviceSpec device_spec) { - lock_guard lock(audio_mutex); + lock_guard lock(audio_mutex); reset_resampler_mutex_held(device_spec); } @@ -159,14 +159,17 @@ void AudioMixer::reset_alsa_mutex_held(DeviceSpec device_spec) } } -void AudioMixer::add_audio(DeviceSpec device_spec, const uint8_t *data, unsigned num_samples, AudioFormat audio_format, int64_t frame_length) +bool AudioMixer::add_audio(DeviceSpec device_spec, const uint8_t *data, unsigned num_samples, AudioFormat audio_format, int64_t frame_length) { AudioDevice *device = find_audio_device(device_spec); - lock_guard lock(audio_mutex); + unique_lock lock(audio_mutex, defer_lock); + if (!lock.try_lock_for(chrono::milliseconds(10))) { + return false; + } if (device->resampling_queue == nullptr) { // No buses use this device; throw it away. - return; + return true; } unsigned num_channels = device->interesting_channels.size(); @@ -200,16 +203,20 @@ void AudioMixer::add_audio(DeviceSpec device_spec, const uint8_t *data, unsigned int64_t local_pts = device->next_local_pts; device->resampling_queue->add_input_samples(local_pts / double(TIMEBASE), audio.data(), num_samples); device->next_local_pts = local_pts + frame_length; + return true; } -void AudioMixer::add_silence(DeviceSpec device_spec, unsigned samples_per_frame, unsigned num_frames, int64_t frame_length) +bool AudioMixer::add_silence(DeviceSpec device_spec, unsigned samples_per_frame, unsigned num_frames, int64_t frame_length) { AudioDevice *device = find_audio_device(device_spec); - lock_guard lock(audio_mutex); + unique_lock lock(audio_mutex, defer_lock); + if (!lock.try_lock_for(chrono::milliseconds(10))) { + return false; + } if (device->resampling_queue == nullptr) { // No buses use this device; throw it away. - return; + return true; } unsigned num_channels = device->interesting_channels.size(); @@ -223,6 +230,7 @@ void AudioMixer::add_silence(DeviceSpec device_spec, unsigned samples_per_frame, // is always the same. device->next_local_pts += frame_length; } + return true; } AudioMixer::AudioDevice *AudioMixer::find_audio_device(DeviceSpec device) @@ -290,7 +298,7 @@ vector AudioMixer::get_output(double pts, unsigned num_samples, Resamplin map> samples_card; vector samples_bus; - lock_guard lock(audio_mutex); + lock_guard lock(audio_mutex); // Pick out all the interesting channels from all the cards. // TODO: If the card has been hotswapped, the number of channels @@ -443,7 +451,7 @@ vector AudioMixer::get_output(double pts, unsigned num_samples, Resamplin map AudioMixer::get_devices() const { - lock_guard lock(audio_mutex); + lock_guard lock(audio_mutex); return get_devices_mutex_held(); } @@ -473,13 +481,13 @@ void AudioMixer::set_name(DeviceSpec device_spec, const string &name) { AudioDevice *device = find_audio_device(device_spec); - lock_guard lock(audio_mutex); + lock_guard lock(audio_mutex); device->name = name; } void AudioMixer::set_input_mapping(const InputMapping &new_input_mapping) { - lock_guard lock(audio_mutex); + lock_guard lock(audio_mutex); map> interesting_channels; for (const InputMapping::Bus &bus : new_input_mapping.buses) { @@ -511,6 +519,6 @@ void AudioMixer::set_input_mapping(const InputMapping &new_input_mapping) InputMapping AudioMixer::get_input_mapping() const { - lock_guard lock(audio_mutex); + lock_guard lock(audio_mutex); return input_mapping; } diff --git a/audio_mixer.h b/audio_mixer.h index 23b020d..6e1d5a9 100644 --- a/audio_mixer.h +++ b/audio_mixer.h @@ -78,9 +78,14 @@ public: ~AudioMixer(); void reset_resampler(DeviceSpec device_spec); - // frame_length is in TIMEBASE units. - void add_audio(DeviceSpec device_spec, const uint8_t *data, unsigned num_samples, bmusb::AudioFormat audio_format, int64_t frame_length); - void add_silence(DeviceSpec device_spec, unsigned samples_per_frame, unsigned num_frames, int64_t frame_length); + // Add audio (or silence) to the given device's queue. Can return false if + // the lock wasn't successfully taken; if so, you should simply try again. + // (This is to avoid a deadlock where a card hangs on the mutex in add_audio() + // while we are trying to shut it down from another thread that also holds + // the mutex.) frame_length is in TIMEBASE units. + bool add_audio(DeviceSpec device_spec, const uint8_t *data, unsigned num_samples, bmusb::AudioFormat audio_format, int64_t frame_length); + bool add_silence(DeviceSpec device_spec, unsigned samples_per_frame, unsigned num_frames, int64_t frame_length); + std::vector get_output(double pts, unsigned num_samples, ResamplingQueue::RateAdjustmentPolicy rate_adjustment_policy); // See comments inside get_output(). @@ -219,7 +224,7 @@ private: unsigned num_cards; - mutable std::mutex audio_mutex; + mutable std::timed_mutex audio_mutex; AudioDevice video_cards[MAX_VIDEO_CARDS]; // Under audio_mutex. diff --git a/mixer.cpp b/mixer.cpp index 9eed47e..38578f9 100644 --- a/mixer.cpp +++ b/mixer.cpp @@ -391,7 +391,10 @@ void Mixer::bm_frame(unsigned card_index, uint16_t timecode, fprintf(stderr, "Card %d dropped %d frame(s) (before timecode 0x%04x), inserting silence.\n", card_index, dropped_frames, timecode); - audio_mixer.add_silence(device, silence_samples, dropped_frames, frame_length); + bool success; + do { + success = audio_mixer.add_silence(device, silence_samples, dropped_frames, frame_length); + } while (!success); } audio_mixer.add_audio(device, audio_frame.data + audio_offset, num_samples, audio_format, frame_length); -- 2.39.2