diff --git a/call/call_config.h b/call/call_config.h index 7fc4b868cc..5834175761 100644 --- a/call/call_config.h +++ b/call/call_config.h @@ -15,7 +15,6 @@ #include "api/rtcerror.h" #include "api/transport/network_control.h" #include "call/audio_state.h" -#include "rtc_base/platform_file.h" namespace webrtc { diff --git a/call/video_receive_stream.h b/call/video_receive_stream.h index d299cd8bde..49efe4fcea 100644 --- a/call/video_receive_stream.h +++ b/call/video_receive_stream.h @@ -28,7 +28,6 @@ #include "common_types.h" // NOLINT(build/include) #include "common_video/include/frame_callback.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" -#include "rtc_base/platform_file.h" namespace webrtc { @@ -228,17 +227,6 @@ class VideoReceiveStream { // TODO(pbos): Add info on currently-received codec to Stats. virtual Stats GetStats() const = 0; - // Takes ownership of the file, is responsible for closing it later. - // Calling this method will close and finalize any current log. - // Giving rtc::kInvalidPlatformFileValue disables logging. - // If a frame to be written would make the log too large the write fails and - // the log is closed and finalized. A |byte_limit| of 0 means no limit. - virtual void EnableEncodedFrameRecording(rtc::PlatformFile file, - size_t byte_limit) = 0; - inline void DisableEncodedFrameRecording() { - EnableEncodedFrameRecording(rtc::kInvalidPlatformFileValue, 0); - } - // RtpDemuxer only forwards a given RTP packet to one sink. However, some // sinks, such as FlexFEC, might wish to be informed of all of the packets // a given sink receives (or any set of sinks). They may do so by registering diff --git a/call/video_send_stream.h b/call/video_send_stream.h index 9bd493307b..3ff624d230 100644 --- a/call/video_send_stream.h +++ b/call/video_send_stream.h @@ -26,7 +26,6 @@ #include "common_types.h" // NOLINT(build/include) #include "common_video/include/frame_callback.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" -#include "rtc_base/platform_file.h" namespace webrtc { diff --git a/media/engine/fakewebrtccall.cc b/media/engine/fakewebrtccall.cc index 0ab39cfbdc..4a923bcb0e 100644 --- a/media/engine/fakewebrtccall.cc +++ b/media/engine/fakewebrtccall.cc @@ -17,7 +17,6 @@ #include "media/base/rtputils.h" #include "rtc_base/checks.h" #include "rtc_base/gunit.h" -#include "rtc_base/platform_file.h" namespace cricket { FakeAudioSendStream::FakeAudioSendStream( @@ -351,11 +350,6 @@ void FakeVideoReceiveStream::SetStats( stats_ = stats; } -void FakeVideoReceiveStream::EnableEncodedFrameRecording(rtc::PlatformFile file, - size_t byte_limit) { - rtc::ClosePlatformFile(file); -} - void FakeVideoReceiveStream::AddSecondarySink( webrtc::RtpPacketSinkInterface* sink) { ++num_added_secondary_sinks_; diff --git a/media/engine/fakewebrtccall.h b/media/engine/fakewebrtccall.h index 5bd335b949..edb3585f3d 100644 --- a/media/engine/fakewebrtccall.h +++ b/media/engine/fakewebrtccall.h @@ -201,9 +201,6 @@ class FakeVideoReceiveStream final : public webrtc::VideoReceiveStream { void SetStats(const webrtc::VideoReceiveStream::Stats& stats); - void EnableEncodedFrameRecording(rtc::PlatformFile file, - size_t byte_limit) override; - void AddSecondarySink(webrtc::RtpPacketSinkInterface* sink) override; void RemoveSecondarySink(const webrtc::RtpPacketSinkInterface* sink) override; diff --git a/modules/video_coding/video_coding_impl.h b/modules/video_coding/video_coding_impl.h index 2e28b0fbe9..75dc889d81 100644 --- a/modules/video_coding/video_coding_impl.h +++ b/modules/video_coding/video_coding_impl.h @@ -130,6 +130,7 @@ class VideoReceiver : public Module { public: VideoReceiver(Clock* clock, EventFactory* event_factory, + // TODO(nisse): Delete. EncodedImageCallback* pre_decode_image_callback, VCMTiming* timing, NackSender* nack_sender = nullptr, diff --git a/video/BUILD.gn b/video/BUILD.gn index ecce1ecb72..3273fda25d 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -95,9 +95,6 @@ rtc_static_library("video") { "../modules/rtp_rtcp", "../modules/utility", "../modules/video_coding", - - # TODO(nisse): Drop this dep, when EnableEncodedFrameRecording is - # deleted also from VideoReceiveStream. "../modules/video_coding:video_coding_utility", "../modules/video_processing", "../rtc_base:rtc_base_approved", @@ -395,7 +392,6 @@ if (rtc_include_tests) { "end_to_end_tests/extended_reports_tests.cc", "end_to_end_tests/fec_tests.cc", "end_to_end_tests/histogram_tests.cc", - "end_to_end_tests/log_tests.cc", "end_to_end_tests/multi_codec_receive_tests.cc", "end_to_end_tests/multi_stream_tester.cc", "end_to_end_tests/multi_stream_tester.h", diff --git a/video/end_to_end_tests/log_tests.cc b/video/end_to_end_tests/log_tests.cc deleted file mode 100644 index 77466b6d8d..0000000000 --- a/video/end_to_end_tests/log_tests.cc +++ /dev/null @@ -1,106 +0,0 @@ -/* - * Copyright 2018 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "media/engine/internaldecoderfactory.h" -#include "modules/video_coding/codecs/vp8/include/vp8.h" -#include "rtc_base/file.h" -#include "test/call_test.h" -#include "test/function_video_encoder_factory.h" -#include "test/testsupport/fileutils.h" - -namespace webrtc { - -class LogEndToEndTest : public test::CallTest { - void SetUp() { paths_.clear(); } - void TearDown() { - for (const auto& path : paths_) { - rtc::RemoveFile(path); - } - } - - public: - int AddFile() { - paths_.push_back(test::TempFilename(test::OutputPath(), "test_file")); - return static_cast(paths_.size()) - 1; - } - - rtc::PlatformFile OpenFile(int idx) { - return rtc::OpenPlatformFile(paths_[idx]); - } - - void LogReceive(bool open) { - if (open) { - video_receive_streams_[0]->EnableEncodedFrameRecording( - OpenFile(AddFile()), 0); - } else { - video_receive_streams_[0]->DisableEncodedFrameRecording(); - } - } - - std::vector paths_; -}; - -TEST_F(LogEndToEndTest, LogsEncodedFramesWhenRequested) { - static const int kNumFramesToRecord = 10; - class LogEncodingObserver : public test::EndToEndTest, - public EncodedFrameObserver { - public: - explicit LogEncodingObserver(LogEndToEndTest* fixture) - : EndToEndTest(kDefaultTimeoutMs), - fixture_(fixture), - encoder_factory_([]() { return VP8Encoder::Create(); }), - recorded_frames_(0) {} - - void PerformTest() override { - fixture_->LogReceive(true); - ASSERT_TRUE(Wait()) << "Timed out while waiting for frame logging."; - } - - void ModifyVideoConfigs( - VideoSendStream::Config* send_config, - std::vector* receive_configs, - VideoEncoderConfig* encoder_config) override { - send_config->post_encode_callback = this; - send_config->rtp.payload_name = "VP8"; - send_config->encoder_settings.encoder_factory = &encoder_factory_; - encoder_config->codec_type = kVideoCodecVP8; - - (*receive_configs)[0].decoders.resize(1); - (*receive_configs)[0].decoders[0].payload_type = - send_config->rtp.payload_type; - (*receive_configs)[0].decoders[0].video_format = - SdpVideoFormat(send_config->rtp.payload_name); - (*receive_configs)[0].decoders[0].decoder_factory = &decoder_factory_; - } - - void EncodedFrameCallback(const EncodedFrame& encoded_frame) override { - rtc::CritScope lock(&crit_); - if (recorded_frames_++ > kNumFramesToRecord) { - fixture_->LogReceive(false); - rtc::File receive_file(fixture_->OpenFile(0)); - uint8_t out[100]; - // If logging has worked correctly file shouldn't be empty, i.e. - // we should be able to read something from them. - EXPECT_LT(0u, receive_file.Read(out, 100)); - observation_complete_.Set(); - } - } - - private: - LogEndToEndTest* const fixture_; - test::FunctionVideoEncoderFactory encoder_factory_; - InternalDecoderFactory decoder_factory_; - rtc::CriticalSection crit_; - int recorded_frames_ RTC_GUARDED_BY(crit_); - } test(this); - - RunBaseTest(&test); -} -} // namespace webrtc diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index e12f2da1eb..0cf303b33b 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -75,6 +75,53 @@ class VideoStreamFactory std::vector streams_; }; +// A decoder wrapper that writes the encoded frames to a file. +class FrameDumpingDecoder : public VideoDecoder { + public: + FrameDumpingDecoder(std::unique_ptr decoder, + rtc::PlatformFile file) + : decoder_(std::move(decoder)), + writer_(IvfFileWriter::Wrap(rtc::File(file), + /* byte_limit= */ 100000000)) {} + + int32_t InitDecode(const VideoCodec* codec_settings, + int32_t number_of_cores) override { + return decoder_->InitDecode(codec_settings, number_of_cores); + } + + int32_t Decode(const EncodedImage& input_image, + bool missing_frames, + const CodecSpecificInfo* codec_specific_info, + int64_t render_time_ms) override { + writer_->WriteFrame(input_image, codec_specific_info->codecType); + + return decoder_->Decode(input_image, missing_frames, codec_specific_info, + render_time_ms); + } + + int32_t RegisterDecodeCompleteCallback( + DecodedImageCallback* callback) override { + return decoder_->RegisterDecodeCompleteCallback(callback); + } + + int32_t Release() override { return decoder_->Release(); } + + // Returns true if the decoder prefer to decode frames late. + // That is, it can not decode infinite number of frames before the decoded + // frame is consumed. + bool PrefersLateDecoding() const override { + return decoder_->PrefersLateDecoding(); + } + + const char* ImplementationName() const override { + return decoder_->ImplementationName(); + } + + private: + std::unique_ptr decoder_; + std::unique_ptr writer_; +}; + // An encoder wrapper that writes the encoded frames to file, one per simulcast // layer. class FrameDumpingEncoder : public VideoEncoder, private EncodedImageCallback { @@ -84,7 +131,7 @@ class FrameDumpingEncoder : public VideoEncoder, private EncodedImageCallback { : encoder_(std::move(encoder)) { for (rtc::PlatformFile file : files) { writers_.push_back( - IvfFileWriter::Wrap(rtc::File(file), 100000000 /* byte_limit */)); + IvfFileWriter::Wrap(rtc::File(file), /* byte_limit= */ 100000000)); } } // Implement VideoEncoder @@ -168,6 +215,14 @@ std::unique_ptr VideoQualityTest::CreateVideoDecoder( } else { decoder = internal_decoder_factory_.CreateVideoDecoder(format); } + if (!params_.logging.encoded_frame_base_path.empty()) { + rtc::StringBuilder str; + str << receive_logs_++; + std::string path = + params_.logging.encoded_frame_base_path + "." + str.str() + ".recv.ivf"; + decoder = absl::make_unique( + std::move(decoder), rtc::CreatePlatformFile(path)); + } return decoder; } @@ -1094,9 +1149,6 @@ void VideoQualityTest::RunWithAnalyzer(const Params& params) { video_capturers_[video_idx].get(), degradation_preference_); } - StartEncodedFrameLogs( - video_receive_streams_[params_.ss[0].selected_stream]); - if (params_.audio.enabled) { SetupAudio(send_transport.get()); StartAudioStreams(); @@ -1308,8 +1360,6 @@ void VideoQualityTest::RunWithRenderers(const Params& params) { SetupAudio(send_transport.get()); } - for (VideoReceiveStream* receive_stream : video_receive_streams_) - StartEncodedFrameLogs(receive_stream); Start(); }); @@ -1330,14 +1380,4 @@ void VideoQualityTest::RunWithRenderers(const Params& params) { }); } -void VideoQualityTest::StartEncodedFrameLogs(VideoReceiveStream* stream) { - if (!params_.logging.encoded_frame_base_path.empty()) { - rtc::StringBuilder str; - str << receive_logs_++; - std::string path = - params_.logging.encoded_frame_base_path + "." + str.str() + ".recv.ivf"; - stream->EnableEncodedFrameRecording(rtc::CreatePlatformFile(path), - 100000000); - } -} } // namespace webrtc diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index 0ff9716b5d..9fc424c85a 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -31,7 +31,6 @@ #include "modules/video_coding/include/video_coding.h" #include "modules/video_coding/jitter_estimator.h" #include "modules/video_coding/timing.h" -#include "modules/video_coding/utility/ivf_file_writer.h" #include "modules/video_coding/utility/vp8_header_parser.h" #include "rtc_base/checks.h" #include "rtc_base/location.h" @@ -131,7 +130,7 @@ VideoReceiveStream::VideoReceiveStream( call_stats_(call_stats), rtp_receive_statistics_(ReceiveStatistics::Create(clock_)), timing_(new VCMTiming(clock_)), - video_receiver_(clock_, nullptr, this, timing_.get(), this, this), + video_receiver_(clock_, nullptr, nullptr, timing_.get(), this, this), stats_proxy_(&config_, clock_), rtp_video_stream_receiver_(&transport_adapter_, call_stats, @@ -313,24 +312,6 @@ VideoReceiveStream::Stats VideoReceiveStream::GetStats() const { return stats_proxy_.GetStats(); } -void VideoReceiveStream::EnableEncodedFrameRecording(rtc::PlatformFile file, - size_t byte_limit) { - { - rtc::CritScope lock(&ivf_writer_lock_); - if (file == rtc::kInvalidPlatformFileValue) { - ivf_writer_.reset(); - } else { - ivf_writer_ = IvfFileWriter::Wrap(rtc::File(file), byte_limit); - } - } - - if (file != rtc::kInvalidPlatformFileValue) { - // Make a keyframe appear as early as possible in the logs, to give actually - // decodable output. - RequestKeyFrame(); - } -} - void VideoReceiveStream::AddSecondarySink(RtpPacketSinkInterface* sink) { rtp_video_stream_receiver_.AddSecondarySink(sink); } @@ -361,25 +342,6 @@ void VideoReceiveStream::OnFrame(const VideoFrame& video_frame) { stats_proxy_.OnRenderedFrame(video_frame); } -// TODO(asapersson): Consider moving callback from video_encoder.h or -// creating a different callback. -EncodedImageCallback::Result VideoReceiveStream::OnEncodedImage( - const EncodedImage& encoded_image, - const CodecSpecificInfo* codec_specific_info, - const RTPFragmentationHeader* fragmentation) { - { - rtc::CritScope lock(&ivf_writer_lock_); - if (ivf_writer_.get()) { - RTC_DCHECK(codec_specific_info); - bool ok = ivf_writer_->WriteFrame(encoded_image, - codec_specific_info->codecType); - RTC_DCHECK(ok); - } - } - - return Result(Result::OK, encoded_image.Timestamp()); -} - void VideoReceiveStream::SendNack( const std::vector& sequence_numbers) { rtp_video_stream_receiver_.RequestPacketRetransmit(sequence_numbers); diff --git a/video/video_receive_stream.h b/video/video_receive_stream.h index fb0ee3bd34..ae5ced51c3 100644 --- a/video/video_receive_stream.h +++ b/video/video_receive_stream.h @@ -32,7 +32,6 @@ namespace webrtc { class CallStats; -class IvfFileWriter; class ProcessThread; class RTPFragmentationHeader; class RtpStreamReceiverInterface; @@ -45,7 +44,6 @@ namespace internal { class VideoReceiveStream : public webrtc::VideoReceiveStream, public rtc::VideoSinkInterface, - public EncodedImageCallback, public NackSender, public KeyFrameRequestSender, public video_coding::OnCompleteFrameCallback, @@ -73,26 +71,12 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, webrtc::VideoReceiveStream::Stats GetStats() const override; - // Takes ownership of the file, is responsible for closing it later. - // Calling this method will close and finalize any current log. - // Giving rtc::kInvalidPlatformFileValue disables logging. - // If a frame to be written would make the log too large the write fails and - // the log is closed and finalized. A |byte_limit| of 0 means no limit. - void EnableEncodedFrameRecording(rtc::PlatformFile file, - size_t byte_limit) override; - void AddSecondarySink(RtpPacketSinkInterface* sink) override; void RemoveSecondarySink(const RtpPacketSinkInterface* sink) override; // Implements rtc::VideoSinkInterface. void OnFrame(const VideoFrame& video_frame) override; - // Implements EncodedImageCallback. - EncodedImageCallback::Result OnEncodedImage( - const EncodedImage& encoded_image, - const CodecSpecificInfo* codec_specific_info, - const RTPFragmentationHeader* fragmentation) override; - // Implements NackSender. void SendNack(const std::vector& sequence_numbers) override; @@ -147,9 +131,6 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, // moved to the new VideoStreamDecoder. std::vector> video_decoders_; - rtc::CriticalSection ivf_writer_lock_; - std::unique_ptr ivf_writer_ RTC_GUARDED_BY(ivf_writer_lock_); - // Members for the new jitter buffer experiment. std::unique_ptr jitter_estimator_; std::unique_ptr frame_buffer_;