Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use device pixel ratio to calculate size of portal for max representation size #1030

Closed
wants to merge 1 commit into from
Closed

Conversation

nickygerritsen
Copy link

This uses window.devicePixelRatio (if set) to calculate the element width and height.

This makes sure that devices that have a device-pixel-ratio of more than 1 (e.g. Retina iOS / Mac devices) will get the correct quality video. See also #1028 where I suggested doing this.

@nickygerritsen
Copy link
Author

Any opinions on this?

@dsparacio
Copy link
Contributor

@nickygerritsen Sorry it is not that I missed this PR I was seeing if @LloydW93 who implemented the ratio limit stuff had any comments. It may be too late unless there is great argument to push this into 2.0 so it may need to move to 2.1. Is that I big setback for you?

@LloydW93
Copy link
Member

LloydW93 commented Feb 9, 2016

Sorry - Unlike Dan I did forget about this.

I think at our end we think this is obviously a useful feature to have, but that a feature toggle would be useful. We'd need to have lots of conversations internally before we would actually be able to turn it on in our use case, but this should not be a reason to hold it back generally.

@dsparacio
Copy link
Contributor

Thanks @LloydW93 . I agree a way to disable this with API at media player is crucial. Now the question is 2.0 or 2.1.

@nickygerritsen
Copy link
Author

I could try to modify it to have an option, similar to the option to use viewport size?

@dsparacio
Copy link
Contributor

If you mod it to have option and rebase it could be put in. t is only tuesday morning here in US but I worry about other timezones and testing and voting to try to make a Friday deadline. I am ok if others are. Looks like we may need a new RC today anyway so if you can do this soon, why not? It has been in the queue well before the deadline for PR.....

@nickygerritsen
Copy link
Author

Hmmm it is evening here in CET, so I can not do it within the next 12 hours....

@dsparacio
Copy link
Contributor

Ah was not aware you are in CET sorry my bad. not sure if we can keep pushing non critical stuff at this point. This CR has had conflicts for a bit and we should of visited this at RC1 or PR deadline last week. My bad for not admining this correctly but my gut feeling is it should wait until 2.1 and be first in.

@nickygerritsen
Copy link
Author

No problem. What is the ETA on 2.1? I can still try to add the options, but then it is maybe better to re-start from the current master?

@dsparacio
Copy link
Contributor

NO official timeline yet but we want it out by NAB in mid- April and we want to starting doing faster releaser cycles around 6 weeks. This all lines up nicely.

@nickygerritsen
Copy link
Author

OK then I know that I don't have to rush too much for now for this to be in 2.1 :). i.e. no tomorrow-first-thing-thingy

LloydW93 pushed a commit to bbc/dash.js that referenced this pull request May 31, 2016
…devicePixelRatio is taken into account which is defaulted to false.
@dsparacio
Copy link
Contributor

Closing covered with PR #1429

@dsparacio dsparacio closed this Jun 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants