☠☠ backed out by e8054b77177a ☠ ☠ | |
author | Andreas Pehrson <apehrson@mozilla.com> |
Wed, 19 Aug 2020 12:48:09 +0000 | |
changeset 545348 | 658ba8f39abe4553e0c06706b867fc86f9239625 |
parent 545347 | 8e67fe040e4a789fb23f8f7fd1efbff781b49b4a |
child 545349 | 28c4e8c373f08bceba73d69cdefa24193a79b05b |
push id | 37713 |
push user | abutkovits@mozilla.com |
push date | Thu, 20 Aug 2020 09:32:09 +0000 |
treeherder | mozilla-central@8cb700c12bd3 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | jib |
bugs | 1652884 |
milestone | 81.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/dom/media/VideoFrameConverter.h +++ b/dom/media/VideoFrameConverter.h @@ -134,24 +134,32 @@ class VideoFrameConverter { [self = RefPtr<VideoFrameConverter>(this), this, aTrackEnabled] { if (mTrackEnabled == aTrackEnabled) { return; } MOZ_LOG(gVideoFrameConverterLog, LogLevel::Debug, ("VideoFrameConverter Track is now %s", aTrackEnabled ? "enabled" : "disabled")); mTrackEnabled = aTrackEnabled; - if (!aTrackEnabled && mLastFrameConverted) { - // After disabling, we re-send the last frame as black in case the - // source had already stopped and no frame is coming soon. - ProcessVideoFrame( - FrameToProcess{nullptr, TimeStamp::Now(), - gfx::IntSize(mLastFrameConverted->width(), - mLastFrameConverted->height()), - true}); + if (!aTrackEnabled) { + // After disabling we immediately send a frame as black, so it can + // be seen quickly, even if no frames are flowing. + if (mLastFrameConverted) { + // This track has already seen frames so we re-send the last one + // as black. + ProcessVideoFrame( + FrameToProcess{nullptr, TimeStamp::Now(), + gfx::IntSize(mLastFrameConverted->width(), + mLastFrameConverted->height()), + true}); + } else { + // This track has not yet seen any frame. We make one up. + ProcessVideoFrame(FrameToProcess{nullptr, TimeStamp::Now(), + gfx::IntSize(640, 480), true}); + } } })); MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv)); Unused << rv; } void AddListener(const RefPtr<VideoConverterListener>& aListener) { nsresult rv = mTaskQueue->Dispatch(NS_NewRunnableFunction(
--- a/dom/media/VideoOutput.h +++ b/dom/media/VideoOutput.h @@ -184,24 +184,33 @@ class VideoOutput : public DirectMediaTr void NotifyEnabledStateChanged(MediaTrackGraph* aGraph, bool aEnabled) override { MutexAutoLock lock(mMutex); mEnabled = aEnabled; DropPastFrames(); if (!mEnabled || mFrames.Length() > 1) { // Re-send frames when disabling, as new frames may not arrive. When // enabling we keep them black until new frames arrive, or re-send if we - // already have frames in the future. + // already have frames in the future. If we're disabling and there are no + // frames available yet, we invent one. Unfortunately with a hardcoded + // size. // // Since mEnabled will affect whether // frames are real, or black, we assign new FrameIDs whenever we re-send // frames after an mEnabled change. for (auto& idChunkPair : mFrames) { idChunkPair.first = mVideoFrameContainer->NewFrameID(); } + if (mFrames.IsEmpty()) { + VideoSegment v; + v.AppendFrame(nullptr, gfx::IntSize(640, 480), PRINCIPAL_HANDLE_NONE, + true, TimeStamp::Now()); + mFrames.AppendElement(std::make_pair(mVideoFrameContainer->NewFrameID(), + *v.GetLastChunk())); + } SendFramesEnsureLocked(); } } Mutex mMutex; TimeStamp mLastFrameTime; // Once the frame is forced to black, we initialize mBlackImage for use in any // following forced-black frames.
--- a/dom/media/gtest/TestVideoTrackEncoder.cpp +++ b/dom/media/gtest/TestVideoTrackEncoder.cpp @@ -1274,16 +1274,56 @@ TEST(VP8VideoTrackEncoder, DisableBetwee // [50ms, 100ms) EXPECT_EQ(PR_USEC_PER_SEC / 1000 * 50UL, frames[1]->mDuration); // [100ms, 200ms) EXPECT_EQ(PR_USEC_PER_SEC / 1000 * 100UL, frames[2]->mDuration); } +// Test that an encoding which is disabled before the first frame becomes black +// immediately. +TEST(VP8VideoTrackEncoder, DisableBeforeFirstFrame) +{ + TestVP8TrackEncoder encoder; + YUVBufferGenerator generator; + generator.Init(mozilla::gfx::IntSize(640, 480)); + nsTArray<RefPtr<EncodedFrame>> frames; + TimeStamp now = TimeStamp::Now(); + + // Disable the track at t=0. + // Pass a frame in at t=50ms. + // Enable the track at t=100ms. + // Stop encoding at t=200ms. + // Should yield 2 frames, 1 black [0, 100); 1 real [100, 200). + + VideoSegment segment; + segment.AppendFrame(generator.GenerateI420Image(), generator.GetSize(), + PRINCIPAL_HANDLE_NONE, false, + now + TimeDuration::FromMilliseconds(50)); + + encoder.SetStartOffset(now); + encoder.Disable(now); + encoder.AppendVideoSegment(std::move(segment)); + + encoder.Enable(now + TimeDuration::FromMilliseconds(100)); + encoder.AdvanceCurrentTime(now + TimeDuration::FromMilliseconds(200)); + encoder.NotifyEndOfStream(); + ASSERT_TRUE(NS_SUCCEEDED(encoder.GetEncodedTrack(frames))); + EXPECT_TRUE(encoder.IsEncodingComplete()); + + ASSERT_EQ(2UL, frames.Length()); + + // [0, 100ms) + EXPECT_EQ(PR_USEC_PER_SEC / 1000 * 100UL, frames[0]->mDuration); + + // [100ms, 200ms) + EXPECT_EQ(PR_USEC_PER_SEC / 1000 * 100UL, frames[1]->mDuration); +} + // Test that an encoding which is enabled on a frame timestamp encodes // frames as expected. TEST(VP8VideoTrackEncoder, EnableOnFrameTime) { TestVP8TrackEncoder encoder; YUVBufferGenerator generator; generator.Init(mozilla::gfx::IntSize(640, 480)); nsTArray<RefPtr<EncodedFrame>> frames;