Fix an out-of-bounds write (found by asan).

[nageru] / video_encoder.cpp

diff --git a/video_encoder.cpp b/video_encoder.cpp
index 96d493252a02e1946363d552bf9908326803b7ee..7719bbbc15c86d1ce2890a64e4117f7935c85a77 100644 (file)
--- a/video_encoder.cpp
+++ b/video_encoder.cpp
@@ -61,30 +61,55 @@ VideoEncoder::VideoEncoder(ResourcePool *resource_pool, QSurface *surface, const
 VideoEncoder::~VideoEncoder()
 {
        quicksync_encoder.reset(nullptr);
-       close_output_stream();
+       while (quicksync_encoders_in_shutdown.load() > 0) {
+               usleep(10000);
+       }
 }
 
 void VideoEncoder::do_cut(int frame)
 {
        string filename = generate_local_dump_filename(frame);
        printf("Starting new recording: %s\n", filename.c_str());
-       quicksync_encoder->shutdown();
+
+       // Do the shutdown of the old encoder in a separate thread, since it can
+       // take some time (it needs to wait for all the frames in the queue to be
+       // done encoding, for one) and we are running on the main mixer thread.
+       // However, since this means both encoders could be sending packets at
+       // 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);
+       QuickSyncEncoder *old_encoder = quicksync_encoder.release();  // When we go C++14, we can use move capture instead.
+       thread([old_encoder, this]{
+               old_encoder->shutdown();
+               stream_mux->unplug();
+
+               // We cannot delete the encoder here, as this thread has no OpenGL context.
+               // We'll deal with it in begin_frame().
+               lock_guard<mutex> lock(qs_mu);
+               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->set_stream_mux(stream_mux.get());
 }
 
 void VideoEncoder::add_audio(int64_t pts, std::vector<float> audio)
 {
+       lock_guard<mutex> lock(qs_mu);
        quicksync_encoder->add_audio(pts, audio);
 }
 
 bool VideoEncoder::begin_frame(GLuint *y_tex, GLuint *cbcr_tex)
 {
+       lock_guard<mutex> lock(qs_mu);
+       qs_needing_cleanup.clear();  // Since we have an OpenGL context here, and are called regularly.
        return quicksync_encoder->begin_frame(y_tex, cbcr_tex);
 }
 
 RefCountedGLsync VideoEncoder::end_frame(int64_t pts, int64_t duration, const std::vector<RefCountedFrame> &input_frames)
 {
+       lock_guard<mutex> lock(qs_mu);
        return quicksync_encoder->end_frame(pts, duration, input_frames);
 }
 
@@ -118,11 +143,6 @@ void VideoEncoder::open_output_stream()
        stream_mux_header.clear();
 }
 
-void VideoEncoder::close_output_stream()
-{
-       stream_mux.reset();
-}
-
 int VideoEncoder::write_packet_thunk(void *opaque, uint8_t *buf, int buf_size)
 {
        VideoEncoder *video_encoder = (VideoEncoder *)opaque;