Closed
Bug 1211658
Opened 6 years ago
Closed 6 years ago
GUM constraints for screen sharing don't affect framerate
Categories
(Core :: WebRTC: Audio/Video, defect)
Core
WebRTC: Audio/Video
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: steinbrecher.johann, Assigned: steinbrecher.johann)
References
Details
Attachments
(1 file, 1 obsolete file)
3.51 KB,
patch
|
drno
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/45.0.2454.101 Safari/537.36 Steps to reproduce: call navigator.mozGetUserMedia with the constraints object. For example { video: { mediaSource: 'screen', require: [ 'width', 'height', 'frameRate' ], width: { min: '160', max: '400' }, height: { min: '90', max: '400' }, frameRate: { min: '5', max: '10' } } } Actual results: The video stream starts capturing from the mediasource screen. The framerate is set 3fps instead of the framerate passed in with the GUM constraints. Expected results: The video stream starts capturing from the mediasource screen. The framerate is set to the framerate passed in with the GUM constraints.
Assignee | ||
Updated•6 years ago
|
Summary: GUM Constraints for screen sharing don't affect framerate → GUM constraints for screen sharing don't affect framerate
Updated•6 years ago
|
Updated•6 years ago
|
Assignee: nobody → steinbrecher.johann
Updated•6 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 1•6 years ago
|
||
Attachment #8669973 -
Flags: review+
Comment 2•6 years ago
|
||
Comment on attachment 8669973 [details] [diff] [review] Proposed fix. We should probably go through the motions of review here even though I've looked at previous versions of this patch.
Attachment #8669973 -
Flags: review+ → review?(jib)
Comment 3•6 years ago
|
||
Comment on attachment 8669973 [details] [diff] [review] Proposed fix. Review of attachment 8669973 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/webrtc/MediaEngineRemoteVideoSource.cpp @@ +391,5 @@ > + /** > + * TASKS todo: > + * - check the mediasource screen|application|tab|window. > + * i.e. MediaEngineRemoteVideoSource->MediaSource and MediaEngineRemoteVideoSource->CaptureEngine > + * - parse contraints and set capabilities This comment can be removed now I think. @@ +407,5 @@ > + case dom::MediaSourceEnum::Application: { > + FlattenedConstraints c(aConstraints); > + mCapability.width = c.mWidth.Clamp(c.mWidth.mIdeal.WasPassed() ? c.mWidth.mIdeal.Value() : aPrefs.mWidth); > + mCapability.height = c.mHeight.Clamp(c.mHeight.mIdeal.WasPassed() ? c.mHeight.mIdeal.Value() : aPrefs.mHeight); > + mCapability.maxFPS = c.mFrameRate.Clamp(c.mFrameRate.mIdeal.WasPassed() ? c.mFrameRate.mIdeal.Value() : aPrefs.mFPS); Long lines should be word-wrapped to 80 characters (not a hard rule, but these are too long for sure).
Attachment #8669973 -
Flags: review?(jib) → review+
Assignee | ||
Comment 4•6 years ago
|
||
Incorporated comments from jib's review.
Updated•6 years ago
|
Attachment #8669973 -
Attachment is obsolete: true
Comment 5•6 years ago
|
||
Comment on attachment 8670040 [details] [diff] [review] Bug fix Carrying forward r+=jib
Attachment #8670040 -
Flags: review+
Comment 6•6 years ago
|
||
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b4eb41c6228 Hans you should check the results above some time tomorrow and see if you find any problems related to your change.
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 7•6 years ago
|
||
dry run results look good.
https://hg.mozilla.org/mozilla-central/rev/a915b7fdf7ac
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Updated•6 years ago
|
Component: Untriaged → WebRTC: Audio/Video
Product: Firefox → Core
Target Milestone: Firefox 44 → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•