[linux] Add mutex for |frames_| in ffmpeg decoder (youtube#524) (yout…

…ube#544) |frames_| might be accessed from different threads. It would lead to unexpected behaviors without thread safe guard. b/280825438 (cherry picked from commit de467c6) Co-authored-by: Jason <[email protected]>
kaidokert · Jun 6, 2023 · d82aead · d82aead
1 parent c700250
commit d82aead
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 5 deletions.
diff --git a/starboard/shared/ffmpeg/ffmpeg_video_decoder_impl.cc b/starboard/shared/ffmpeg/ffmpeg_video_decoder_impl.cc
@@ -207,6 +207,7 @@ void VideoDecoderImpl<FFMPEG>::Reset() {
     InitializeCodec();
   }
 
+  ScopedLock lock(decode_target_and_frames_mutex_);
   decltype(frames_) frames;
   frames_ = std::queue<scoped_refptr<CpuVideoFrame>>();
 }
@@ -310,6 +311,7 @@ bool VideoDecoderImpl<FFMPEG>::DecodePacket(AVPacket* packet) {
 
   bool result = true;
   if (output_mode_ == kSbPlayerOutputModeDecodeToTexture) {
+    ScopedLock lock(decode_target_and_frames_mutex_);
     frames_.push(frame);
   }
 
@@ -396,7 +398,7 @@ void VideoDecoderImpl<FFMPEG>::TeardownCodec() {
   ffmpeg_->FreeFrame(&av_frame_);
 
   if (output_mode_ == kSbPlayerOutputModeDecodeToTexture) {
-    ScopedLock lock(decode_target_mutex_);
+    ScopedLock lock(decode_target_and_frames_mutex_);
     if (SbDecodeTargetIsValid(decode_target_)) {
       DecodeTargetRelease(decode_target_graphics_context_provider_,
                           decode_target_);
@@ -411,7 +413,7 @@ SbDecodeTarget VideoDecoderImpl<FFMPEG>::GetCurrentDecodeTarget() {
 
   // We must take a lock here since this function can be called from a
   // separate thread.
-  ScopedLock lock(decode_target_mutex_);
+  ScopedLock lock(decode_target_and_frames_mutex_);
   while (frames_.size() > 1 && frames_.front()->HasOneRef()) {
     frames_.pop();
   }

diff --git a/starboard/shared/ffmpeg/ffmpeg_video_decoder_impl.h b/starboard/shared/ffmpeg/ffmpeg_video_decoder_impl.h
@@ -143,9 +143,8 @@ class VideoDecoderImpl<FFMPEG> : public VideoDecoder {
   // GetCurrentDecodeTarget() needs to be called from an arbitrary thread
   // to obtain the current decode target (which ultimately ends up being a
   // copy of |decode_target_|), we need to safe-guard access to |decode_target_|
-  // and we do so through this mutex.
-  Mutex decode_target_mutex_;
-  // Mutex frame_mutex_;
+  // and |frames_|, we do so through this mutex.
+  Mutex decode_target_and_frames_mutex_;
 
   // int frame_last_rendered_pts_;
   // scoped_refptr<VideoFrame> frame_;