1008935 - Loop desktop UI needs to handle different aspect ratios better

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Mark Banner (:standard8)]

Using my Android phone (Galaxy S4 mini) as a link clicker, when I view the video on the desktop, the video is taller than the conversation window, and requires scrolling to see the local video.

This also applies when the conversation window is expanded out into a separate window.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

This is using Firefox for Android, I assume?

Comment 2

[Mark Banner (:standard8)]

Firefox for Android was the link clicker end. No idea if Chrome works there yet or not, but I suspect the aspect ratio issue would be the same.

Comment 3

[Sevaan Franks [:sevaan]]

Attachment: Portrait (Scrolling)

Attached image illustrates the problem.

Comment 4

[Sevaan Franks [:sevaan]]

Attachment: Landscape (No Scrolling)

Attachment: When the phone is rotated to landscape, scrolling is no longer an issue.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

I haven't looked into this particular case, but it's at least conceivable that fixing <https://bugzilla.mozilla.org/show_bug.cgi?id=624647> would make fixing this easier.

Comment 6

[Mark Banner (:standard8)]

I'm actually suspecting that some of this could be due to the way the TokBox sdk is handling this, or our interactions with it.

Comment 7

[mozilla-support]

One suggestion is that you can create a container element for the subscribed stream, styled and positioned the way you want it to. Then when you call session.subscribe(...,..., properties), you can specify:
insertMode: "append" => append the stream element to the container you have created
width: "100%" => the stream element will fill up container's width
height: "100%" => the stream element will fill up container's height

see if that helps?

Comment 8

[Sevaan Franks [:sevaan]]

Thanks for jumping in, Tokbox. Can an engineer confirm whether this helps?

Comment 9

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Attachment: tb_inline-styles.png

(In reply to mozilla-support from comment #7)
>
> see if that helps?

Thanks. Well, even with insertMode set to "append", inline styles are style applied to the DOM wrappers generated by the SDK — see attached capture — which makes styling any parent container element rather hard, to say the least.

Is there a way to prevent these styles to be applied inline? to be applied at all?

Comment 10

[mozilla-support]

The best practice is to not add any styling to SDK generated elements. Best practice is to set insertMode to append in the JS and the SDK will append a generated element to your own container element (which the SDK will not touch). When you create the subscriber in the JS and pass in width:100% and height:100%, the SDK generated elements should fill your container 100%. If you keep css styling to your own elements, the inline styling applied to SDK generated elements should not be causing problems.

Comment 11

[Sevaan Franks [:sevaan]]

Can someone clarify whether this is a UX/Design issue, or whether there is an engineering solution to this bug?

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

There is inter-related UX and engineering work here. 

There are a variety of things we could consider doing when we receive portrait video that is taller than the receiving window:

* add a vertical scrollbar (current behavior)

* crop the top and bottom of the video until the height matches that of landscape mode, using existing window size

* scale down the height of the video to match the window size.  Either accept that we will have pillarboxing, or resize the window horizontally.

* force the window to be tall enough to (within some bound) to accomodate the incoming video.  Consider making the window narrower at the same time, so that we don't need to have blank bars on the sides ("pillarboxing").

There are other variants as well, but these are the most likely.

Given the current constraints posed by the SDK, it's not entirely clear to me which ones of these are easy/hard/possible.  Perhaps :NiKo` knows.  I think that having bug 624647 implemented in Gecko is very likely to make more things easier or even possible, so I've added that as a dependency.

Comment 13

[Sevaan Franks [:sevaan]]

(In reply to Dan Mosedale (:dmose - needinfo? me for responses) from comment #12)

Thanks for putting your thoughts down, dmose.

Out of those options, my original thinking was to scaling down the height of the video to match the window size. I'm okay with the pillarboxing (learned a new word) as it just shows you're chatting with someone using their phone.

:NiKo, if you could let us know your thoughts, that would be awesome.

Thanks, all!

Comment 14

[Romain Testard [:RT]]

Attachment: Skype_mobile_to_desktop.png

For what it's worth Skype implements "scale down the height of the video to match the window size.  Either accept that we will have pillarboxing, or resize the window horizontally."

Another thing we may want to consider is that the mobile user will turn his phone and go "portrait mode" whilst on the call.

Right now on Loop when a link clicker is on FF for ANdroid, the conversation window on the browser client side displays a vertical scrolbar and adapts automatically as the mobile goes portrait mode.

Comment 15

[song]

I think suggestions in this thread can help with this issue: https://bugzilla.mozilla.org/show_bug.cgi?id=1020445#c15

When user goes to landscape mode from portrait mode or vice versa, there will be a streamPropertyChanged event fired with new width/height and you can adjust accordingly.

Comment 16

[Sevaan Franks [:sevaan]]

Attachment: Example of how video should be resized in portrait and landscape.

Hi all,

Ideally we should be resizing the link clicker's video to fit the Loop window.

If the link clicker is using their phone in portrait mode, the height of the video should match the height of the window, with the width scaled to the correct proportions.

If the link clicker is using their phone in landscape mode, the width of the video should match the width of the window, with the height scaled to the correct proportions.

Any unused space will be kept dark.

Niko, can you confirm that this is feasible?

Comment 17

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

(In reply to Sevaan Franks [:sevaan] from comment #16)
> Niko, can you confirm that this is feasible?

It *should*, though with using the SDK it might get complicated because it generates the video elements and associated styles for us, so that makes us overriding styles and/or styling these elements using Javascript, where size & aspect ratio computation is easier.

needinfo Tokbox for thoughts here.

Also a remark regarding the provided mockup: as both standalone and desktop full screen conversation window will use a fluid layout (resizable browser windows), we probably want to ensure this strategy keeps matching all possible use cases (eg. I remember we went with falling back to responsive side-by-side video display for wide fullscreen conversation window in Talkilla.)

Comment 18

[song]

First, you can detect if stream is in portrait or landscape mode with the stream videoDimensions property by comparing width/height: http://tokbox.com/opentok/libraries/client/js/reference/Stream.html

Sometimes, the user might the phone in the middle of the call. You can detect changes to/from landscape to portrait by listening to the streamPropertyChanged event and checking the stream's videoDimensions: http://tokbox.com/opentok/libraries/client/js/reference/StreamPropertyChangedEvent.html

After identifying the stream dimensions, you can style the video:

* If the link clicker is using their phone in portrait mode, the height of the video should match the height of the window, with the width scaled to the correct proportions.
    
    function updateStreamHeight(stream){
      var dim = stream.videoDimensions;
      var width = (100*dim.width)/dim.height; // retrieve height percentage: ?/100% = dim.width/dim.height
      $(".remote_wrapper").css("padding-bottom", "100%");
      $(".remote_wrapper").css("width", width+"%");
    }


* If the link clicker is using their phone in landscape mode, the width of the video should match the width of the window, with the height scaled to the correct proportions.
    
    function updateStreamHeight(stream){
      var dim = stream.videoDimensions;
      var height = (100*dim.height)/dim.width; // retrieve height percentage: 100%/? = dim.width/dim.height
      $(".remote_wrapper").css("padding-bottom", height+"%");
      $(".remote_wrapper").css("width", "100%");
    }

Sample HTML for this example:
  <div class="conversation">
    <div class="media nested">
      <div class="remote_wrapper">
        <div class="remote"></div>  
      </div>      
      <div class="local"></div>
    </div>
  </div>

Sample CSS for this example: 
.conversation .media.nested {
  position: relative;
  text-align:center;
}
 
.conversation .media.nested .remote {
  display: inline-block;
  position: absolute;
  top: 0; bottom: 0; left: 0; right: 0;
  background: #000;
}
    
.conversation .media.nested .remote_wrapper {
  display: inline-block;
  position: relative;
  background: #0f0;
  width: 60%;
  padding-bottom: 45%;
}
 
.conversation .media.nested .local {
  display: inline-block;
  position: absolute;
  bottom: 4px;
  right: 0;
  width: 30%;
  max-width: 140px;
  height: 75px;
  z-index: 2;
  background-color:#ff0;
}

gist: https://gist.github.com/songz/c7fbd10cb6389b7c2cab

Comment 19

[mozilla-support]

^^^

Comment 20

[Sevaan Franks [:sevaan]]

Thanks, Song. Niko, shall we mark this bug as resolved and move work to an implementation bug?

Comment 21

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

(In reply to Sevaan Franks [:sevaan] from comment #20)
> Thanks, Song. Niko, shall we mark this bug as resolved and move work to an
> implementation bug?

Sure.

Comment 22

[Sevaan Franks [:sevaan]]

Implementation work to take place in Bug 1046789.