twilio / twilio-video.js

Twilio’s Programmable Video JavaScript SDK
https://www.twilio.com/docs/video/javascript
Other
567 stars 215 forks source link

Pass in contraints for localMedia #47

Closed ZuSe closed 7 years ago

ZuSe commented 7 years ago

Hi folks,

i am running into some problems when I try to force a specific video resolution. I am creating my localMedia-object like this:

return Twilio.Video.LocalMedia.getLocalMedia(this.E_VIDEO_SETTINGS).then((localMedia: any) => { where E_VIDEO_SETTINGS is defined as

E_VIDEO_SETTINGS = {
    audio: true,
    video: {
      width: {ideal: 1280, max: 1920},
      height: {ideal: 720, max: 1080}
    }
  };

From what I see in your code this should be piped through the whole process and be passed in to the native getUserMedia-Api as contraints object. Nvtl. my resulting mediaStream instance is 4:3 and low res.

Any ideas what goes wrong here (my webcam supports 720p)?

Best, Patrick

Tested on: Chrome 56 and Vivialdi 1.6 on Ubuntu 16.04

markandrus commented 7 years ago

Hi @ZuSe,

I tested this out in isolation of twilio-video.js using the following code:

const videoConstraints = {
  width: {
    ideal: 1280,
    max: 1920
  },
  height: {
    ideal: 720,
    max: 1080
  }
};

function testVideoDimensions(videoConstraints) {
  return navigator.mediaDevices.getUserMedia({
    video: videoConstraints
  }).then(stream => {
    const track = stream.getVideoTracks()[0];
    const video = document.createElement('video');
    video.srcObject = new webkitMediaStream([track]);
    return new Promise((resolve, reject) => {
      video.onloadeddata = () => {
        resolve({
          width: video.videoWidth,
          height: video.videoHeight
        });
      };
      video.onerror = reject;
    }).then(dimensions => {
      track.stop();
      return dimensions;
    }, error => {
      track.stop();
      throw error;
    });
  });
}

testVideoDimensions(videoConstraints).then(dimensions => {
  console.log(dimensions);
}, error => {
  console.error(error);
});

I consistently receive a 640 by 480 video, so my guess is that these constraints are incorrect. I will try to find working constraints.

Best, Mark

markandrus commented 7 years ago

@ZuSe if I pass min instead of ideal, I get 1280 by 720. I will need to read up on the ideal algorithm and browser support.

markandrus commented 7 years ago

Hey @ZuSe,

This appears to be a Chrome bug. I've found two tickets that relate to this issue, namely

  1. Issue 657733: Implement selectSettings algorithm for MediaStreamTrack constraint resolution
  2. Issue 682887: getUserMedia gives 640x480 when constraints say 1280x720 (blocked on Issue 657733)

So they support the ideal syntax, but it doesn't do anything yet internally. Luckily they have started on the issue. In the meantime, the only workaround I can imagine is to try with min values and back-off if getLocalMedia (really getUserMedia) rejects.

ZuSe commented 7 years ago

Hey @markandrus

since upgrading from 4 to 5 (also on 7 now) it's not even working on Firefox that accepted it before. Don't you pass the object any longer through to MediaDevices.getUserMedia() ?

My updated code looks like, according to MDN (https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getUserMedia) this should trigger the same behavior as ideal:

  private E_VIDEO_SETTINGS = {
    video: {
      width: 1280,
      height: 720
    }
  };

public getLocalVideoTrack(): Promise<any> {
    if (!this.localVideoTrack) {
      return Twilio.Video.createLocalVideoTrack(this.E_VIDEO_SETTINGS).then((localVideoTrack: any) => {
        this.localVideoTrack = localVideoTrack;
        return localVideoTrack;
      });
    }
    return Promise.resolve(this.localVideoTrack);

  }

Ubuntu 17.04, Firefox 53

manjeshbhargav commented 7 years ago

Hi @ZuSe ,

Your video constraints should be: {width: 1280, height: 720}· createLocalVideoTrack accepts only the video constraints. Let me know if it works for you.

Thanks,

Manjesh

ZuSe commented 7 years ago

@manjeshbhargav

works, thanks. Please update your docs accordingly when you have some time :)

manjeshbhargav commented 7 years ago

Hi @ZuSe ,

Glad that you're unblocked. I think the docs for createLocalVideoTrack is already correct. It accepts a CreateLocalTrackOptions object, which is essentially a MediaTrackConstraints object: https://media.twiliocdn.com/sdk/js/video/releases/1.0.0-beta7/docs/global.html#createLocalVideoTrack

Manjesh

ZuSe commented 7 years ago

I think we can close this. It's also going to be supported by Chrome 60+

av-k commented 7 years ago

Hi @manjeshbhargav, Have some keys, if i use navigator.mediaDevices.getUserMedia (constraints={video:{deviceId: {exact: device.deviceId}}}) and i need max video resolution, how i can doit this with createLocalVideoTrack? Are you thinking about extending the settings?

ZuSe commented 7 years ago

@av-k

check my code above You can just pass in a javascript object with height and width attribute:

{ height: { min:xx, ideal:yy, max:zz}, width: {...} }

av-k commented 7 years ago

@ZuSe hi, you inattentively read my message, I need in addition to the dimensions, use selected device that performs stream ({deviceId: {exact: device.deviceId}})

markandrus commented 7 years ago

Hi @av-k,

I can't speak for @ZuSe, but I think he meant you that you can take the union of the two objects. So for example, in addition to providing a deviceId, you could also provide height and width in the same object:

{
  deviceId: {
    exact: device.deviceId
  },
  height: {
    min: minHeight,
    max: maxHeight,
    ideal: idealHeight
  },
  width: {
    min: minWidth,
    max: maxWidth,
    ideal: idealWidth
  }
}

This object that you pass to createLocalVideoTrack should match the MediaTrackConstraints type. (If you call createLocalTracks, you'll need to pass a MediaStreamConstraints object instead.)

av-k commented 7 years ago

@markandrus thanks for answer, im has already implemented this logic by passingMediaStream(with props) instance tocreateLocalVideoTrack. I just wanted to make sure that he really takes all of these options (forcreateLocalVideoTrack`). Thx.