diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 4005de896b..dc404dc13f 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -3012,7 +3012,6 @@ bool WebRtcVideoChannel::WebRtcVideoReceiveStream::ReconfigureCodecs( } if (codec.ulpfec.red_rtx_payload_type != -1) { - // TODO(tommi): Look into if/when this happens in practice. rtx_associated_payload_types[codec.ulpfec.red_rtx_payload_type] = codec.ulpfec.red_payload_type; } diff --git a/modules/video_coding/decoder_database.cc b/modules/video_coding/decoder_database.cc index 882de27489..15d8dcc339 100644 --- a/modules/video_coding/decoder_database.cc +++ b/modules/video_coding/decoder_database.cc @@ -19,13 +19,14 @@ VCMDecoderDataBase::VCMDecoderDataBase() { decoder_sequence_checker_.Detach(); } -bool VCMDecoderDataBase::DeregisterExternalDecoder(uint8_t payload_type) { +VideoDecoder* VCMDecoderDataBase::DeregisterExternalDecoder( + uint8_t payload_type) { RTC_DCHECK_RUN_ON(&decoder_sequence_checker_); auto it = decoders_.find(payload_type); if (it == decoders_.end()) { - // Not found. - return false; + return nullptr; } + // We can't use payload_type to check if the decoder is currently in use, // because payload type may be out of date (e.g. before we decode the first // frame after RegisterReceiveCodec). @@ -33,8 +34,9 @@ bool VCMDecoderDataBase::DeregisterExternalDecoder(uint8_t payload_type) { // Release it if it was registered and in use. current_decoder_ = absl::nullopt; } + VideoDecoder* ret = it->second; decoders_.erase(it); - return true; + return ret; } // Add the external decoder object to the list of external decoders. diff --git a/modules/video_coding/decoder_database.h b/modules/video_coding/decoder_database.h index 98bbf851d6..59b683b42a 100644 --- a/modules/video_coding/decoder_database.h +++ b/modules/video_coding/decoder_database.h @@ -30,7 +30,9 @@ class VCMDecoderDataBase { VCMDecoderDataBase& operator=(const VCMDecoderDataBase&) = delete; ~VCMDecoderDataBase() = default; - bool DeregisterExternalDecoder(uint8_t payload_type); + // Returns a pointer to the previously registered decoder or nullptr if none + // was registered for the `payload_type`. + VideoDecoder* DeregisterExternalDecoder(uint8_t payload_type); void RegisterExternalDecoder(uint8_t payload_type, VideoDecoder* external_decoder); bool IsExternalDecoderRegistered(uint8_t payload_type) const; diff --git a/modules/video_coding/video_receiver2.cc b/modules/video_coding/video_receiver2.cc index fe35420c73..36f53a70a4 100644 --- a/modules/video_coding/video_receiver2.cc +++ b/modules/video_coding/video_receiver2.cc @@ -13,8 +13,10 @@ #include #include +#include #include +#include "absl/algorithm/container.h" #include "api/video_codecs/video_codec.h" #include "api/video_codecs/video_decoder.h" #include "modules/video_coding/decoder_database.h" @@ -32,9 +34,8 @@ VideoReceiver2::VideoReceiver2(Clock* clock, VCMTiming* timing, const FieldTrialsView& field_trials) : clock_(clock), - timing_(timing), - decodedFrameCallback_(timing_, clock_, field_trials), - codecDataBase_() { + decoded_frame_callback_(timing, clock_, field_trials), + codec_database_() { decoder_sequence_checker_.Detach(); } @@ -45,29 +46,38 @@ VideoReceiver2::~VideoReceiver2() { // Register a receive callback. Will be called whenever there is a new frame // ready for rendering. int32_t VideoReceiver2::RegisterReceiveCallback( - VCMReceiveCallback* receiveCallback) { + VCMReceiveCallback* receive_callback) { RTC_DCHECK_RUN_ON(&construction_sequence_checker_); // This value is set before the decoder thread starts and unset after // the decoder thread has been stopped. - decodedFrameCallback_.SetUserReceiveCallback(receiveCallback); + decoded_frame_callback_.SetUserReceiveCallback(receive_callback); return VCM_OK; } -void VideoReceiver2::RegisterExternalDecoder(VideoDecoder* externalDecoder, - uint8_t payloadType) { +void VideoReceiver2::RegisterExternalDecoder( + std::unique_ptr decoder, + uint8_t payload_type) { RTC_DCHECK_RUN_ON(&decoder_sequence_checker_); - RTC_DCHECK(decodedFrameCallback_.UserReceiveCallback()); + RTC_DCHECK(decoded_frame_callback_.UserReceiveCallback()); - if (externalDecoder == nullptr) { - codecDataBase_.DeregisterExternalDecoder(payloadType); + if (decoder) { + RTC_DCHECK(!codec_database_.IsExternalDecoderRegistered(payload_type)); + codec_database_.RegisterExternalDecoder(payload_type, decoder.get()); + video_decoders_.push_back(std::move(decoder)); } else { - codecDataBase_.RegisterExternalDecoder(payloadType, externalDecoder); + VideoDecoder* registered = + codec_database_.DeregisterExternalDecoder(payload_type); + if (registered) { + video_decoders_.erase(absl::c_find_if( + video_decoders_, + [registered](const auto& d) { return d.get() == registered; })); + } } } -bool VideoReceiver2::IsExternalDecoderRegistered(uint8_t payloadType) const { +bool VideoReceiver2::IsExternalDecoderRegistered(uint8_t payload_type) const { RTC_DCHECK_RUN_ON(&decoder_sequence_checker_); - return codecDataBase_.IsExternalDecoderRegistered(payloadType); + return codec_database_.IsExternalDecoderRegistered(payload_type); } // Must be called from inside the receive side critical section. @@ -76,7 +86,7 @@ int32_t VideoReceiver2::Decode(const VCMEncodedFrame* frame) { TRACE_EVENT0("webrtc", "VideoReceiver2::Decode"); // Change decoder if payload type has changed. VCMGenericDecoder* decoder = - codecDataBase_.GetDecoder(*frame, &decodedFrameCallback_); + codec_database_.GetDecoder(*frame, &decoded_frame_callback_); if (decoder == nullptr) { return VCM_NO_CODEC_REGISTERED; } @@ -89,7 +99,7 @@ void VideoReceiver2::RegisterReceiveCodec( uint8_t payload_type, const VideoDecoder::Settings& settings) { RTC_DCHECK_RUN_ON(&construction_sequence_checker_); - codecDataBase_.RegisterReceiveCodec(payload_type, settings); + codec_database_.RegisterReceiveCodec(payload_type, settings); } } // namespace webrtc diff --git a/modules/video_coding/video_receiver2.h b/modules/video_coding/video_receiver2.h index 86f7f55096..cbc7885227 100644 --- a/modules/video_coding/video_receiver2.h +++ b/modules/video_coding/video_receiver2.h @@ -11,6 +11,9 @@ #ifndef MODULES_VIDEO_CODING_VIDEO_RECEIVER2_H_ #define MODULES_VIDEO_CODING_VIDEO_RECEIVER2_H_ +#include +#include + #include "api/field_trials_view.h" #include "api/sequence_checker.h" #include "api/video_codecs/video_decoder.h" @@ -37,24 +40,27 @@ class VideoReceiver2 { void RegisterReceiveCodec(uint8_t payload_type, const VideoDecoder::Settings& decoder_settings); - void RegisterExternalDecoder(VideoDecoder* externalDecoder, - uint8_t payloadType); - bool IsExternalDecoderRegistered(uint8_t payloadType) const; - int32_t RegisterReceiveCallback(VCMReceiveCallback* receiveCallback); + void RegisterExternalDecoder(std::unique_ptr decoder, + uint8_t payload_type); - int32_t Decode(const webrtc::VCMEncodedFrame* frame); + bool IsExternalDecoderRegistered(uint8_t payload_type) const; + int32_t RegisterReceiveCallback(VCMReceiveCallback* receive_callback); + + int32_t Decode(const VCMEncodedFrame* frame); private: SequenceChecker construction_sequence_checker_; SequenceChecker decoder_sequence_checker_; Clock* const clock_; - VCMTiming* timing_; - VCMDecodedFrameCallback decodedFrameCallback_; + VCMDecodedFrameCallback decoded_frame_callback_; + // Holds/owns the decoder instances that are registered via + // `RegisterExternalDecoder` and referenced by `codec_database_`. + std::vector> video_decoders_; // Callbacks are set before the decoder thread starts. // Once the decoder thread has been started, usage of `_codecDataBase` moves // over to the decoder thread. - VCMDecoderDataBase codecDataBase_; + VCMDecoderDataBase codec_database_; }; } // namespace webrtc diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index 94dedaa6fc..0c2ab0f07f 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -602,8 +602,7 @@ void VideoReceiveStream2::CreateAndRegisterExternalDecoder( std::move(video_decoder), FileWrapper::OpenWriteOnly(ssb.str())); } - video_decoders_.push_back(std::move(video_decoder)); - video_receiver_.RegisterExternalDecoder(video_decoders_.back().get(), + video_receiver_.RegisterExternalDecoder(std::move(video_decoder), decoder.payload_type); } diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h index 753d5bbc42..9d4cac76e1 100644 --- a/video/video_receive_stream2.h +++ b/video/video_receive_stream2.h @@ -276,10 +276,6 @@ class VideoReceiveStream2 std::unique_ptr video_stream_decoder_; RtpStreamsSynchronizer rtp_stream_sync_; - // TODO(nisse, philipel): Creation and ownership of video encoders should be - // moved to the new VideoStreamDecoder. - std::vector> video_decoders_; - std::unique_ptr buffer_; std::unique_ptr media_receiver_