Move audio encoding over to its own mutex, again reducing mutex contention.

diff --git a/quicksync_encoder.cpp b/quicksync_encoder.cpp
index 88d0fb401d514ff7422e1239a007e7d206eda319..f7e1696ab9f13419f182ed03a556e2e913c2c1b1 100644 (file)
--- 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<float> audio)
 {
+       lock_guard<mutex> 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<mutex> 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,
-               { &current_file_mux_metrics, &total_mux_metrics }));
+       {
+               lock_guard<mutex> 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,
+                       { &current_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 f4e9e0b57e37c890b44536a24e86babd9d2dd558..de243ac6f2a3f28e98a9a30ae090a90b35332a67 100644 (file)
--- 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<float> audio);
-       bool is_zerocopy() const;
+       void add_audio(int64_t pts, std::vector<float> 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<RefCountedFrame> &input_frames, GLuint *y_tex, GLuint *cbcr_tex);

diff --git a/quicksync_encoder_impl.h b/quicksync_encoder_impl.h
index 85ac3ab980d525369e7f9c788d31dd0b8208ce78..bc84e0adc7280beadb9e11ad1ee278eec59ac1ec 100644 (file)
--- a/quicksync_encoder_impl.h
+++ b/quicksync_encoder_impl.h
@@ -162,6 +162,7 @@ private:
        std::map<int, PendingFrame> reorder_buffer;
        int quicksync_encoding_frame_num = 0;
 
+       std::mutex file_audio_encoder_mutex;
        std::unique_ptr<AudioEncoder> file_audio_encoder;
 
        X264Encoder *x264_encoder;  // nullptr if not using x264.

diff --git a/video_encoder.cpp b/video_encoder.cpp
index 61a1305c2dbd1e986a9ab78eb452d90859879925..efc9732c654093f4a00240039b5de55527382862 100644 (file)
--- 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<mutex> lock(qs_mu);
+       lock(qs_mu, qs_audio_mu);
+       lock_guard<mutex> 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<float> 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<mutex> lock(qs_mu);
+               lock_guard<mutex> 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<float> 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 31c0c5f05ff41cb5a97510a0a50e2d240442dcc2..21595a380c992b7df76c0d3531e0cee72b5b2cfe 100644 (file)
--- 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<QuickSyncEncoder> quicksync_encoder;  // Under <qs_mu>.
+       mutable std::mutex qs_mu, qs_audio_mu;
+       std::unique_ptr<QuickSyncEncoder> quicksync_encoder;  // Under <qs_mu> _and_ <qs_audio_mu>.
        movit::ResourcePool *resource_pool;
        QSurface *surface;
        std::string va_display;