Bug 1299515 - Clean up MediaEngineRemoteVideoSource::FrameSizeChange. r?jib
When looking for whether MediaEngineRemoteVideoSource would ever set the size
to 0x0 I found FrameSizeChange() hard to read. This attempts to clean it up,
and adds some invariant checks to see that we're not doing unnecessary work.
MozReview-Commit-ID: KPCTNQelh2a
--- a/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
+++ b/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
@@ -1,15 +1,17 @@
/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "MediaEngineRemoteVideoSource.h"
+#include <limits>
+
#include "mozilla/RefPtr.h"
#include "VideoUtils.h"
#include "nsIPrefService.h"
#include "MediaTrackConstraints.h"
#include "CamerasChild.h"
extern mozilla::LogModule* GetMediaManagerLog();
#define LOG(msg) MOZ_LOG(GetMediaManagerLog(), mozilla::LogLevel::Debug, msg)
@@ -408,45 +410,58 @@ MediaEngineRemoteVideoSource::NotifyPull
StreamTime delta = aDesiredTime - aSource->GetEndOfAppendedData(aID);
if (delta > 0) {
AppendToTrack(aSource, mImage, aID, delta, aPrincipalHandle);
}
}
void
-MediaEngineRemoteVideoSource::FrameSizeChange(unsigned int w, unsigned int h)
+MediaEngineRemoteVideoSource::FrameSizeChange(unsigned int aWidth, unsigned int aHeight)
{
- if ((mWidth < 0) || (mHeight < 0) ||
- (w != (unsigned int) mWidth) || (h != (unsigned int) mHeight)) {
- LOG(("MediaEngineRemoteVideoSource Video FrameSizeChange: %ux%u was %ux%u", w, h, mWidth, mHeight));
- mWidth = w;
- mHeight = h;
+ if (aWidth == static_cast<unsigned int>(mWidth) &&
+ aHeight == static_cast<unsigned int>(mHeight)) {
+ MOZ_ASSERT_UNREACHABLE("Unexpected frame size change that didn't change");
+ return;
+ }
+
+ if (aWidth == 0 || aHeight == 0) {
+ MOZ_ASSERT_UNREACHABLE("We'd rather not get a frame if it's 0 pixels large");
+ return;
+ }
- auto settings = mSettings;
- NS_DispatchToMainThread(media::NewRunnableFrom([settings, w, h]() mutable {
- settings->mWidth.Value() = w;
- settings->mHeight.Value() = h;
- return NS_OK;
- }));
- }
+ MOZ_RELEASE_ASSERT(aWidth <= std::numeric_limits<decltype(mWidth)>::max() &&
+ aHeight <= std::numeric_limits<decltype(mHeight)>::max(),
+ "Overflow");
+
+ LOG(("MediaEngineRemoteVideoSource Video FrameSizeChange: %dx%d to %ux%u", mWidth, mHeight, aWidth, aHeight));
+ mWidth = aWidth;
+ mHeight = aHeight;
+
+ NS_DispatchToMainThread(media::NewRunnableFrom([settings = mSettings,
+ aWidth, aHeight]() mutable {
+ settings->mWidth.Value() = aWidth;
+ settings->mHeight.Value() = aHeight;
+ return NS_OK;
+ }));
}
int
MediaEngineRemoteVideoSource::DeliverFrame(uint8_t* aBuffer ,
- const camera::VideoFrameProperties& aProps)
+ const camera::VideoFrameProperties& aProps)
{
// Check for proper state.
if (mState != kStarted) {
LOG(("DeliverFrame: video not started"));
return 0;
}
- // Update the dimensions
- FrameSizeChange(aProps.width(), aProps.height());
+ if (aProps.width() != mWidth || aProps.height() != mHeight) {
+ FrameSizeChange(aProps.width(), aProps.height());
+ }
layers::PlanarYCbCrData data;
RefPtr<layers::PlanarYCbCrImage> image;
{
// We grab the lock twice, but don't hold it across the (long) CopyData
MonitorAutoLock lock(mMonitor);
if (!mImageContainer) {
LOG(("DeliverFrame() called after Stop()!"));