GUM constraints for screen sharing don't affect framerate

RESOLVED FIXED

Status

()

Core
WebRTC: Audio/Video
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: steinbrecher.johann, Assigned: steinbrecher.johann)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(firefox44 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
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

2 years ago
Depends on: 1121047
(Assignee)

Updated

2 years ago
Summary: GUM Constraints for screen sharing don't affect framerate → GUM constraints for screen sharing don't affect framerate
Blocks: 1121047
No longer depends on: 1121047
Assignee: nobody → steinbrecher.johann
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 1

2 years ago
Created attachment 8669973 [details] [diff] [review]
Proposed fix.
Attachment #8669973 - Flags: review+
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 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

2 years ago
Created attachment 8670040 [details] [diff] [review]
Bug fix

Incorporated comments from jib's review.
Attachment #8669973 - Attachment is obsolete: true
Comment on attachment 8670040 [details] [diff] [review]
Bug fix

Carrying forward r+=jib
Attachment #8670040 - Flags: review+
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

2 years ago
Keywords: checkin-needed
(Assignee)

Comment 7

2 years ago
dry run results look good.
https://hg.mozilla.org/mozilla-central/rev/a915b7fdf7ac
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Component: Untriaged → WebRTC: Audio/Video
Product: Firefox → Core
Target Milestone: Firefox 44 → ---
Duplicate of this bug: 1144912
You need to log in before you can comment on or make changes to this bug.