Especially video resolution.
Created attachment 372437 [details] Patch
Attachment 372437 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:99: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 1 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 372460 [details] Patch
Attachment 372460 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:99: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 372460 [details] Patch Attachment 372460 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/12520994 New failing tests: platform/ios/mediastream/getUserMedia-single-capture.html platform/ios/mediastream/audio-muted-in-background-tab.html platform/ios/mediastream/video-muted-in-background-tab.html
Created attachment 372466 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.14.5
Created attachment 372479 [details] Patch
Attachment 372479 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:99: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 372479 [details] Patch Patch ready to review. The idea behind this patch would be to have RealtimeVideoSource be fully responsible for frame rate/size adaptation. This would remove the need for RealtimeVideoCaptureSource to be a RealtimeMediaSource , which might simplify things. It could instead become a RealtimeCamera or something like that.
Comment on attachment 372479 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372479&action=review > Source/WebCore/platform/mediastream/RealtimeVideoCaptureSource.cpp:185 > + for (auto& size : standardVideoSizes()) { > + if (size.width() < minimumWidth || size.height() < minimumHeight) > + minimumAspectRatio = std::min(minimumAspectRatio, static_cast<double>(size.width()) / size.height()); > + } This isn't new, but I don't think this look is necessary because minimumWidth and minimumHeight are both 1. > Source/WebCore/platform/mediastream/RealtimeVideoCaptureSource.cpp:205 > + const double epsilon = 0.001; > + return frameRate + epsilon >= range.minimum && frameRate - epsilon <= range.maximum; Also not new, but I wonder if we could use WTF::areEssentiallyEqual here? > Source/WebCore/platform/mediastream/RealtimeVideoCaptureSource.cpp:444 > + if (!match) > + return; Logging an error here might be helpful. > Source/WebCore/platform/mediastream/RealtimeVideoSource.cpp:73 > + if (!width) > + width = sourceSize.width() * height.value() / sourceSize.height(); > + m_currentSettings.setWidth(*width); Maybe we should ASSERT(sourceSize.width() && sourceSize.height()) since we will get a divide-by-zero crash if either is zero.
Thanks for the review. (In reply to Eric Carlson from comment #10) > Comment on attachment 372479 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=372479&action=review > > > Source/WebCore/platform/mediastream/RealtimeVideoCaptureSource.cpp:185 > > + for (auto& size : standardVideoSizes()) { > > + if (size.width() < minimumWidth || size.height() < minimumHeight) > > + minimumAspectRatio = std::min(minimumAspectRatio, static_cast<double>(size.width()) / size.height()); > > + } > > This isn't new, but I don't think this look is necessary because > minimumWidth and minimumHeight are both 1. OK, I'll remove this one. > > > Source/WebCore/platform/mediastream/RealtimeVideoCaptureSource.cpp:205 > > + const double epsilon = 0.001; > > + return frameRate + epsilon >= range.minimum && frameRate - epsilon <= range.maximum; > > Also not new, but I wonder if we could use WTF::areEssentiallyEqual here? Probably, I'll do the change in a follow-up. > > Source/WebCore/platform/mediastream/RealtimeVideoCaptureSource.cpp:444 > > + if (!match) > > + return; > > Logging an error here might be helpful. OK > > Source/WebCore/platform/mediastream/RealtimeVideoSource.cpp:73 > > + if (!width) > > + width = sourceSize.width() * height.value() / sourceSize.height(); > > + m_currentSettings.setWidth(*width); > > Maybe we should ASSERT(sourceSize.width() && sourceSize.height()) since we > will get a divide-by-zero crash if either is zero. OK
Created attachment 372561 [details] Patch for landing
Attachment 372561 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:99: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 372561 [details] Patch for landing Clearing flags on attachment: 372561 Committed r246644: <https://trac.webkit.org/changeset/246644>
All reviewed patches have been landed. Closing bug.
<rdar://problem/51953374>