From fb7eac9998ac186130409e859eb7c63906f7a2fd Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Thu, 29 Jun 2017 23:47:34 +0200 Subject: [PATCH] Move audio encoding over to its own mutex, again reducing mutex contention. --- quicksync_encoder.cpp | 19 +++++++++++++------ quicksync_encoder.h | 6 ++++-- quicksync_encoder_impl.h | 1 + video_encoder.cpp | 12 +++++++++--- video_encoder.h | 4 ++-- 5 files changed, 29 insertions(+), 13 deletions(-) diff --git a/quicksync_encoder.cpp b/quicksync_encoder.cpp index 88d0fb4..f7e1696 100644 --- a/quicksync_encoder.cpp +++ b/quicksync_encoder.cpp @@ -1719,6 +1719,7 @@ bool QuickSyncEncoderImpl::begin_frame(int64_t pts, int64_t duration, YCbCrLumaC void QuickSyncEncoderImpl::add_audio(int64_t pts, vector audio) { + lock_guard lock(file_audio_encoder_mutex); assert(!is_shutdown); file_audio_encoder->encode_audio(audio, pts + global_delay()); } @@ -1800,7 +1801,10 @@ void QuickSyncEncoderImpl::shutdown() storage_thread.join(); // Encode any leftover audio in the queues, and also any delayed frames. - file_audio_encoder->encode_last_audio(); + { + lock_guard lock(file_audio_encoder_mutex); + file_audio_encoder->encode_last_audio(); + } if (!global_flags.x264_video_to_disk) { release_encode(); @@ -1837,11 +1841,14 @@ void QuickSyncEncoderImpl::open_output_file(const std::string &filename) current_file_mux_metrics.reset(); - AVCodecParametersWithDeleter audio_codecpar = file_audio_encoder->get_codec_parameters(); - file_mux.reset(new Mux(avctx, frame_width, frame_height, Mux::CODEC_H264, video_extradata, audio_codecpar.get(), TIMEBASE, - std::bind(&DiskSpaceEstimator::report_write, disk_space_estimator, filename, _1), - Mux::WRITE_BACKGROUND, - { ¤t_file_mux_metrics, &total_mux_metrics })); + { + lock_guard lock(file_audio_encoder_mutex); + AVCodecParametersWithDeleter audio_codecpar = file_audio_encoder->get_codec_parameters(); + file_mux.reset(new Mux(avctx, frame_width, frame_height, Mux::CODEC_H264, video_extradata, audio_codecpar.get(), TIMEBASE, + std::bind(&DiskSpaceEstimator::report_write, disk_space_estimator, filename, _1), + Mux::WRITE_BACKGROUND, + { ¤t_file_mux_metrics, &total_mux_metrics })); + } metric_current_file_start_time_seconds = get_timestamp_for_metrics(); if (global_flags.x264_video_to_disk) { diff --git a/quicksync_encoder.h b/quicksync_encoder.h index f4e9e0b..de243ac 100644 --- a/quicksync_encoder.h +++ b/quicksync_encoder.h @@ -59,14 +59,16 @@ class ResourcePool; // This is just a pimpl, because including anything X11-related in a .h file // tends to trip up Qt. All the real logic is in QuickSyncEncoderImpl, // defined in quicksync_encoder_impl.h. +// +// This class is _not_ thread-safe, except where mentioned. class QuickSyncEncoder { public: QuickSyncEncoder(const std::string &filename, movit::ResourcePool *resource_pool, QSurface *surface, const std::string &va_display, int width, int height, AVOutputFormat *oformat, X264Encoder *x264_encoder, DiskSpaceEstimator *disk_space_estimator); ~QuickSyncEncoder(); void set_stream_mux(Mux *mux); // Does not take ownership. Must be called unless x264 is used for the stream. - void add_audio(int64_t pts, std::vector audio); - bool is_zerocopy() const; + void add_audio(int64_t pts, std::vector audio); // Thread-safe. + bool is_zerocopy() const; // Thread-safe. // See VideoEncoder::begin_frame(). bool begin_frame(int64_t pts, int64_t duration, movit::YCbCrLumaCoefficients ycbcr_coefficients, const std::vector &input_frames, GLuint *y_tex, GLuint *cbcr_tex); diff --git a/quicksync_encoder_impl.h b/quicksync_encoder_impl.h index 85ac3ab..bc84e0a 100644 --- a/quicksync_encoder_impl.h +++ b/quicksync_encoder_impl.h @@ -162,6 +162,7 @@ private: std::map reorder_buffer; int quicksync_encoding_frame_num = 0; + std::mutex file_audio_encoder_mutex; std::unique_ptr file_audio_encoder; X264Encoder *x264_encoder; // nullptr if not using x264. diff --git a/video_encoder.cpp b/video_encoder.cpp index 61a1305..efc9732 100644 --- a/video_encoder.cpp +++ b/video_encoder.cpp @@ -96,7 +96,8 @@ void VideoEncoder::do_cut(int frame) // the same time, it means pts could come out of order to the stream mux, // and we need to plug it until the shutdown is complete. stream_mux->plug(); - lock_guard lock(qs_mu); + lock(qs_mu, qs_audio_mu); + lock_guard lock1(qs_mu, adopt_lock), lock2(qs_audio_mu, adopt_lock); QuickSyncEncoder *old_encoder = quicksync_encoder.release(); // When we go C++14, we can use move capture instead. X264Encoder *old_x264_encoder = nullptr; if (global_flags.x264_video_to_disk) { @@ -136,8 +137,11 @@ void VideoEncoder::change_x264_bitrate(unsigned rate_kbit) void VideoEncoder::add_audio(int64_t pts, std::vector audio) { + // Take only qs_audio_mu, since add_audio() is thread safe + // (we can only conflict with do_cut(), which takes qs_audio_mu) + // and we don't want to contend with begin_frame(). { - lock_guard lock(qs_mu); + lock_guard lock(qs_audio_mu); quicksync_encoder->add_audio(pts, audio); } stream_audio_encoder->encode_audio(audio, pts + quicksync_encoder->global_delay()); @@ -146,7 +150,9 @@ void VideoEncoder::add_audio(int64_t pts, std::vector audio) bool VideoEncoder::is_zerocopy() const { // Explicitly do _not_ take qs_mu; this is called from the mixer, - // and qs_mu might be contended. is_zerocopy() is thread safe. + // and qs_mu might be contended. is_zerocopy() is thread safe + // and never called in parallel with do_cut() (both happen only + // from the mixer thread). return quicksync_encoder->is_zerocopy(); } diff --git a/video_encoder.h b/video_encoder.h index 31c0c5f..21595a3 100644 --- a/video_encoder.h +++ b/video_encoder.h @@ -79,8 +79,8 @@ private: int write_packet2(uint8_t *buf, int buf_size, AVIODataMarkerType type, int64_t time); AVOutputFormat *oformat; - mutable std::mutex qs_mu; - std::unique_ptr quicksync_encoder; // Under . + mutable std::mutex qs_mu, qs_audio_mu; + std::unique_ptr quicksync_encoder; // Under _and_ . movit::ResourcePool *resource_pool; QSurface *surface; std::string va_display; -- 2.39.2