Skip to content

Commit

Permalink
[linux] Add mutex for |frames_| in ffmpeg decoder (youtube#524)
Browse files Browse the repository at this point in the history
|frames_| might be accessed from different threads. It would lead to
unexpected behaviors without thread safe guard.

b/280825438
  • Loading branch information
jasonzhangxx committed Jun 1, 2023
1 parent c6c41f2 commit de467c6
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
6 changes: 4 additions & 2 deletions starboard/shared/ffmpeg/ffmpeg_video_decoder_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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>>();
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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_);
Expand All @@ -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();
}
Expand Down
5 changes: 2 additions & 3 deletions starboard/shared/ffmpeg/ffmpeg_video_decoder_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down

0 comments on commit de467c6

Please sign in to comment.