Fixing paused video doesn't react on changing device orientation

[kareljuricka]

Reacting on #194 I found that problem is in condiiton: frame == prev_frame_ in apple_video_renderer.cc. It returns nullptr in this case and renderLoop inside ShakaPlayerView.mm doesn't pass: if (CGImageRef image = _player.videoRenderer->Render(nullptr, &aspect_ratio)) {.. condition

So solution would be remove frame == prev_frame_, but this could brings unnecessary overhead to rendering loop. So I limit this condition only for case, that video is paused. To make it possible, I have to make player_ property protected, to be inheritable from parent class. Also I had to move mutex_ up to player_, due to some compilations errors. This works and bug is fixed.

I could imagine there should be better solution, directly in file ShakaPlayerView.mm - watching screen orientation change and only forcing recalculation in that case. Additionaly storing some informations from last render and use them for paused state. But I don't want to make huge updates in original source code.

Fixes #194

[TheModMaker]

This will have some major performance impact. When we're paused, users would expect the app to be doing (almost) nothing. This would require us to regenerate the same image 30+ times a second. This would be better done in ShakaPlayerView. The orientation and bounds have no effect on this generated image, so you should only have to change the contentsRect and frame fields of the image based on the new bounds. You basically just need to use this code in an event listener:

https://github.com/google/shaka-player-embedded/blob/9fc13728fa004d5928fbe72955ca3ebbae75ed8c/shaka/src/public/ShakaPlayerView.mm#L172-L188

[kareljuricka]

I know it's overhead, but it was the simplest solution.

I tried to think your advice but:

shaka::ShakaRect<uint32_t> image_bounds = {0, 0, CGImageGetWidth(image), 
                                            CGImageGetHeight(image)}; 

returns wrong image size (image size from previous orientation) after resize and I have to call

CGImageRef image = _player.videoRenderer->Render(nullptr, &aspect_ratio)

to get correct new ones.

So how can I do it, without calling Render() method. I understand I can't call render method because frame == prevframe returns nill as result.

[TheModMaker]

The base image is always the same, it is based on the image data without any resizing. We use contentsRect and bounds to resize the image to fit based on the orientation. We want the original image size for image_bounds.

[kareljuricka]

Sorry, you're right. Image dimensions are really same, I was wrong.

I create new commit, where I reverse all previous patches and has working solution as you advice. Two notes:

1. I had to make image or image_bounds as class property to access it in orientationChanged method.. I deciced to image property. I didn't find out way how to access original image from _imageLayer.contents. Maybe (I hope) there is better way?

2. I has to change self.bounds to self.superview.bounds because self.bounds returned wrong values. I made same change also in renderLoop. Then I could remove my workaround in swift project inside my ViewController: self.playerView.frame = self.view.bounds; which was used to fix problem previously (calling on orientation change), but worked only when player was playing.

[kareljuricka]

I updated all points, except superview workaround. I will try to figure out why view.bounds are wrong.

UPDATE: I don't think there is problem with calling it earlier, because in renderLoop, after orientation change, self.bounds results in previous dimensions and do not change to new one. All future rounds of renderLoop then returns wrong bounds. So problem is probably with self.bounds, which aren't implicit recalculated on device orientation change...

[kareljuricka]

Ok, I found better solution: In my own ViewController, in which I inserting ShakaPlayerView, I moved

self.playerView.frame = self.view.bounds;

from viewWillAppear

inside another override function:

override func viewDidLayoutSubviews() {
  if (self.playerView != nil) {
     self.playerView.frame = self.view.bounds;
  }
}

with this solution, I can use ShakaPlayer bounds instead of super view bounds, because they are updated from parent.