Skip to content

Fix video rotation via style param#526

Merged
AlexxIT merged 2 commits intoAlexxIT:masterfrom
dbuezas:fix/rotation-and-digital-ptz
Aug 24, 2023
Merged

Fix video rotation via style param#526
AlexxIT merged 2 commits intoAlexxIT:masterfrom
dbuezas:fix/rotation-and-digital-ptz

Conversation

@dbuezas
Copy link
Copy Markdown
Contributor

@dbuezas dbuezas commented Jul 13, 2023

@AlexxIT
Copy link
Copy Markdown
Owner

AlexxIT commented Jul 13, 2023

Are you sure construction ... can pass ESLint?

@dbuezas
Copy link
Copy Markdown
Contributor Author

dbuezas commented Jul 13, 2023

Safari support:

  • DomPoint on Safari 10.1
  • DomMatrix on Safari 11 (it was called WebKitCSSMatrix before)

@dbuezas
Copy link
Copy Markdown
Contributor Author

dbuezas commented Jul 13, 2023

Are you sure construction ... can pass ESLint?

It shouldn't be an issue: https://caniuse.com/mdn-javascript_operators_spread_spread_in_function_calls

@dbuezas
Copy link
Copy Markdown
Contributor Author

dbuezas commented Jul 13, 2023

I'm also transpiling from typescript to es2015/es6, which begins support on Safari 10, so I would be extremely surprised if there are any syntax issues.

@gtxaspec
Copy link
Copy Markdown

+1 for this, solves video rotation not working with PTZ enabled =D

@gtxaspec
Copy link
Copy Markdown

gtxaspec commented Aug 3, 2023

@dbuezas I applied these changes manually, and rotation + ptz works in Firefox but not Chrome Beta... Version 115.0.5790.98 (Official Build) beta (64-bit), or in the Android HAOS App... wondering if you can test or if it's just me.

@AlexxIT
Copy link
Copy Markdown
Owner

AlexxIT commented Aug 3, 2023

@dbuezas maybe it's easier to implement rotation param for card. So users shouldn't use custom style for that

@dbuezas
Copy link
Copy Markdown
Contributor Author

dbuezas commented Aug 3, 2023

I use chrome and the android app and it works for me. Maybe you haven't cleared cache?

Regarding styling in general, I also use it myself to cut a video in half in a "double lense" camera, so it is quite useful to have it IMO

@gtxaspec
Copy link
Copy Markdown

gtxaspec commented Aug 3, 2023

I use chrome and the android app and it works for me. Maybe you haven't cleared cache?

Regarding styling in general, I also use it myself to cut a video in half in a "double lense" camera, so it is quite useful to have it IMO

You're right! My mistake, it works well!

@AlexxIT AlexxIT merged commit 9f9ca41 into AlexxIT:master Aug 24, 2023
@AlexxIT
Copy link
Copy Markdown
Owner

AlexxIT commented Aug 24, 2023

Thanks!

@dbuezas
Copy link
Copy Markdown
Contributor Author

dbuezas commented Aug 24, 2023

I'm not sure if I left a comment on how the heck this works, so here it is:

  • the user rotated/skewed/cropped (transformed) the video via css
  • The digital ptz needs to know if the browser added black bars to center the video while in full screen, so the same place is zoomed
  • this picks the transform the user did and the original aspect ratio of the video, and figures out the aspect ratio of the bounding box of the transformed video
  • with that and the dimensions of the screen, this figures out where the browser added black bars to center the video
  • Now it can finally ensure that zoom and pan are consistent between the full screen and normal players

@dbuezas
Copy link
Copy Markdown
Contributor Author

dbuezas commented Aug 24, 2023

Oh, and adds a wrapper element around the video so it can manipulate it without interfering with the user styles

@AlexxIT AlexxIT changed the title Add video wrapper for ptz transforms for compatibility with rotations Fix video rotations via style param Aug 24, 2023
@AlexxIT AlexxIT changed the title Fix video rotations via style param Fix video rotation via style param Aug 24, 2023
@AlexxIT AlexxIT added this to the v3.3.0 milestone Aug 24, 2023
@AlexxIT
Copy link
Copy Markdown
Owner

AlexxIT commented Aug 24, 2023

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.

3 participants