Encode audio as soon as we get it, instead of manually interleaving.

I honestly can't remember why this design was chosen back in the day,
but it doesn't appear to make much sense as long as the mux can interleave
for us. It (hopefully) fixes a deadlock I've seen a few times, where the
mixer hangs on begin_frame() because there are no free surfaces,
but saved_codeddata() (which is freeing them, after save) is stuck
waiting for more audio from the mixer.

diff --git a/quicksync_encoder.cpp b/quicksync_encoder.cpp
index 001901873b7fd641a43183f0c7a2b15b45e46ed8..041ac07d85db3076aa8df0bc1b8306a1717d7edb 100644 (file)
--- a/quicksync_encoder.cpp
+++ b/quicksync_encoder.cpp
@@ -194,7 +194,7 @@ FrameReorderer::Frame FrameReorderer::get_first_frame()
 
 class QuickSyncEncoderImpl {
 public:
-       QuickSyncEncoderImpl(const std::string &filename, movit::ResourcePool *resource_pool, QSurface *surface, const string &va_display, int width, int height, AVOutputFormat *oformat, AudioEncoder *stream_audio_encoder, X264Encoder *x264_encoder);
+       QuickSyncEncoderImpl(const std::string &filename, movit::ResourcePool *resource_pool, QSurface *surface, const string &va_display, int width, int height, AVOutputFormat *oformat, X264Encoder *x264_encoder);
        ~QuickSyncEncoderImpl();
        void add_audio(int64_t pts, vector<float> audio);
        bool begin_frame(GLuint *y_tex, GLuint *cbcr_tex);
@@ -206,6 +206,11 @@ public:
                stream_mux = mux;
        }
 
+       // So we never get negative dts.
+       int64_t global_delay() const {
+               return int64_t(ip_period - 1) * (TIMEBASE / MAX_FPS);
+       }
+
 private:
        struct storage_task {
                unsigned long long display_order;
@@ -219,11 +224,6 @@ private:
                int64_t pts, duration;
        };
 
-       // So we never get negative dts.
-       int64_t global_delay() const {
-               return int64_t(ip_period - 1) * (TIMEBASE / MAX_FPS);
-       }
-
        void open_output_file(const std::string &filename);
        void encode_thread_func();
        void encode_remaining_frames_as_p(int encoding_frame_num, int gop_start_display_frame_num, int64_t last_dts);
@@ -231,7 +231,6 @@ private:
        void encode_frame(PendingFrame frame, int encoding_frame_num, int display_frame_num, int gop_start_display_frame_num,
                          int frame_type, int64_t pts, int64_t dts, int64_t duration);
        void storage_task_thread();
-       void encode_remaining_audio();
        void storage_task_enqueue(storage_task task);
        void save_codeddata(storage_task task);
        int render_packedsequence();
@@ -276,12 +275,10 @@ private:
        int current_storage_frame;
 
        map<int, PendingFrame> pending_video_frames;  // under frame_queue_mutex
-       map<int64_t, vector<float>> pending_audio_frames;  // under frame_queue_mutex
        movit::ResourcePool *resource_pool;
        QSurface *surface;
 
        unique_ptr<AudioEncoder> file_audio_encoder;
-       AudioEncoder *stream_audio_encoder;
 
        unique_ptr<FrameReorderer> reorderer;
        X264Encoder *x264_encoder;  // nullptr if not using x264.
@@ -1638,26 +1635,6 @@ void QuickSyncEncoderImpl::save_codeddata(storage_task task)
                        stream_mux->add_packet(pkt, task.pts + global_delay(), task.dts + global_delay());
                }
        }
-       // Encode and add all audio frames up to and including the pts of this video frame.
-       for ( ;; ) {
-               int64_t audio_pts;
-               vector<float> audio;
-               {
-                       unique_lock<mutex> lock(frame_queue_mutex);
-                       frame_queue_nonempty.wait(lock, [this]{ return storage_thread_should_quit || !pending_audio_frames.empty(); });
-                       if (storage_thread_should_quit && pending_audio_frames.empty()) return;
-                       auto it = pending_audio_frames.begin();
-                       if (it->first > task.pts) break;
-                       audio_pts = it->first;
-                       audio = move(it->second);
-                       pending_audio_frames.erase(it); 
-               }
-
-               file_audio_encoder->encode_audio(audio, audio_pts + global_delay());
-               stream_audio_encoder->encode_audio(audio, audio_pts + global_delay());
-
-               if (audio_pts == task.pts) break;
-       }
 }
 
 
@@ -1743,8 +1720,8 @@ namespace {
 
 }  // namespace
 
-QuickSyncEncoderImpl::QuickSyncEncoderImpl(const std::string &filename, movit::ResourcePool *resource_pool, QSurface *surface, const string &va_display, int width, int height, AVOutputFormat *oformat, AudioEncoder *stream_audio_encoder, X264Encoder *x264_encoder)
-       : current_storage_frame(0), resource_pool(resource_pool), surface(surface), stream_audio_encoder(stream_audio_encoder), x264_encoder(x264_encoder), frame_width(width), frame_height(height)
+QuickSyncEncoderImpl::QuickSyncEncoderImpl(const std::string &filename, movit::ResourcePool *resource_pool, QSurface *surface, const string &va_display, int width, int height, AVOutputFormat *oformat, X264Encoder *x264_encoder)
+       : current_storage_frame(0), resource_pool(resource_pool), surface(surface), x264_encoder(x264_encoder), frame_width(width), frame_height(height)
 {
        file_audio_encoder.reset(new AudioEncoder(AUDIO_OUTPUT_CODEC_NAME, DEFAULT_AUDIO_OUTPUT_BIT_RATE, oformat));
        open_output_file(filename);
@@ -1871,11 +1848,7 @@ bool QuickSyncEncoderImpl::begin_frame(GLuint *y_tex, GLuint *cbcr_tex)
 void QuickSyncEncoderImpl::add_audio(int64_t pts, vector<float> audio)
 {
        assert(!is_shutdown);
-       {
-               unique_lock<mutex> lock(frame_queue_mutex);
-               pending_audio_frames[pts] = move(audio);
-       }
-       frame_queue_nonempty.notify_all();
+       file_audio_encoder->encode_audio(audio, pts + global_delay());
 }
 
 RefCountedGLsync QuickSyncEncoderImpl::end_frame(int64_t pts, int64_t duration, const vector<RefCountedFrame> &input_frames)
@@ -1943,7 +1916,9 @@ void QuickSyncEncoderImpl::shutdown()
                storage_task_queue_changed.notify_all();
        }
        storage_thread.join();
-       encode_remaining_audio();
+
+       // Encode any leftover audio in the queues, and also any delayed frames.
+       file_audio_encoder->encode_last_audio();
 
        release_encode();
        deinit_va();
@@ -2050,25 +2025,6 @@ void QuickSyncEncoderImpl::encode_remaining_frames_as_p(int encoding_frame_num,
        }
 }
 
-void QuickSyncEncoderImpl::encode_remaining_audio()
-{
-       // This really ought to be empty by now, but just to be sure...
-       for (auto &pending_frame : pending_audio_frames) {
-               int64_t audio_pts = pending_frame.first;
-               vector<float> audio = move(pending_frame.second);
-
-               file_audio_encoder->encode_audio(audio, audio_pts + global_delay());
-               if (stream_audio_encoder) {
-                       stream_audio_encoder->encode_audio(audio, audio_pts + global_delay());
-               }
-       }
-       pending_audio_frames.clear();
-
-       // Encode any leftover audio in the queues, and also any delayed frames.
-       // Note: stream_audio_encoder is not owned by us, so don't call encode_last_audio().
-       file_audio_encoder->encode_last_audio();
-}
-
 void QuickSyncEncoderImpl::add_packet_for_uncompressed_frame(int64_t pts, int64_t duration, const uint8_t *data)
 {
        AVPacket pkt;
@@ -2191,8 +2147,8 @@ void QuickSyncEncoderImpl::encode_frame(QuickSyncEncoderImpl::PendingFrame frame
 }
 
 // Proxy object.
-QuickSyncEncoder::QuickSyncEncoder(const std::string &filename, movit::ResourcePool *resource_pool, QSurface *surface, const string &va_display, int width, int height, AVOutputFormat *oformat, AudioEncoder *stream_audio_encoder, X264Encoder *x264_encoder)
-       : impl(new QuickSyncEncoderImpl(filename, resource_pool, surface, va_display, width, height, oformat, stream_audio_encoder, x264_encoder)) {}
+QuickSyncEncoder::QuickSyncEncoder(const std::string &filename, movit::ResourcePool *resource_pool, QSurface *surface, const string &va_display, int width, int height, AVOutputFormat *oformat, X264Encoder *x264_encoder)
+       : impl(new QuickSyncEncoderImpl(filename, resource_pool, surface, va_display, width, height, oformat, x264_encoder)) {}
 
 // Must be defined here because unique_ptr<> destructor needs to know the impl.
 QuickSyncEncoder::~QuickSyncEncoder() {}
@@ -2222,3 +2178,6 @@ void QuickSyncEncoder::set_stream_mux(Mux *mux)
        impl->set_stream_mux(mux);
 }
 
+int64_t QuickSyncEncoder::global_delay() const {
+       return impl->global_delay();
+}

diff --git a/quicksync_encoder.h b/quicksync_encoder.h
index 25b19725dee6841faf83c453af49fab38a279ca3..e55fd4dce115f7c6720de791bda963c8e0f89304 100644 (file)
--- a/quicksync_encoder.h
+++ b/quicksync_encoder.h
@@ -55,7 +55,7 @@ class ResourcePool;
 // .cpp file.
 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, AudioEncoder *stream_audio_encoder, X264Encoder *x264_encoder);
+        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);
         ~QuickSyncEncoder();
 
        void set_stream_mux(Mux *mux);  // Does not take ownership. Must be called unless x264 is used for the stream.
@@ -64,6 +64,7 @@ public:
        RefCountedGLsync end_frame(int64_t pts, int64_t duration, const std::vector<RefCountedFrame> &input_frames);
        void shutdown();  // Blocking. Does not require an OpenGL context.
        void release_gl_resources();  // Requires an OpenGL context. Must be run after shutdown.
+       int64_t global_delay() const;  // So we never get negative dts.
 
 private:
        std::unique_ptr<QuickSyncEncoderImpl> impl;

diff --git a/video_encoder.cpp b/video_encoder.cpp
index 7719bbbc15c86d1ce2890a64e4117f7935c85a77..eb1cd670e40c512904cf2e0c2389dfda37e785d1 100644 (file)
--- a/video_encoder.cpp
+++ b/video_encoder.cpp
@@ -50,7 +50,7 @@ VideoEncoder::VideoEncoder(ResourcePool *resource_pool, QSurface *surface, const
        }
 
        string filename = generate_local_dump_filename(/*frame=*/0);
-       quicksync_encoder.reset(new QuickSyncEncoder(filename, resource_pool, surface, va_display, width, height, oformat, stream_audio_encoder.get(), x264_encoder.get()));
+       quicksync_encoder.reset(new QuickSyncEncoder(filename, resource_pool, surface, va_display, width, height, oformat, x264_encoder.get()));
 
        open_output_stream();
        stream_audio_encoder->add_mux(stream_mux.get());
@@ -90,7 +90,7 @@ void VideoEncoder::do_cut(int frame)
                qs_needing_cleanup.emplace_back(old_encoder);
        }).detach();
 
-       quicksync_encoder.reset(new QuickSyncEncoder(filename, resource_pool, surface, va_display, width, height, oformat, stream_audio_encoder.get(), x264_encoder.get()));
+       quicksync_encoder.reset(new QuickSyncEncoder(filename, resource_pool, surface, va_display, width, height, oformat, x264_encoder.get()));
        quicksync_encoder->set_stream_mux(stream_mux.get());
 }
 
@@ -98,6 +98,7 @@ void VideoEncoder::add_audio(int64_t pts, std::vector<float> audio)
 {
        lock_guard<mutex> lock(qs_mu);
        quicksync_encoder->add_audio(pts, audio);
+       stream_audio_encoder->encode_audio(audio, pts + quicksync_encoder->global_delay());
 }
 
 bool VideoEncoder::begin_frame(GLuint *y_tex, GLuint *cbcr_tex)