SRGSSR / srgletterbox-apple

The official SRG SSR media playback experience
https://srgssr.github.io/marketing/letterbox/
MIT License
14 stars 7 forks source link

Switching between different letterbox views #57

Open vpdn opened 7 years ago

vpdn commented 7 years ago

Issue overview

Check the correct option below

Description of the problem

The SRF App shows videos in a feed. When tapped, that video is then played in fullscreen. The way we try to implement it with Letterbox is to use two separate LetterboxViews, one for the feed and another for the fullscreen player. Those two players share the same letterboxController.

It seems that the controller reference must be nilled out on the letterbox view when the controller is added to another letterboxView. Otherwise the behaviour seems rather random.

One of the following would be good: 1) the behaviour is documented somewhere 2) auto nil letterbox views that have the controller associated 3) allow multiple letterbox views to be linked to the same letterbox controller

Code sample

I've attached an additional demo into the demo app.

srgletterbox-ios-multiview.zip

Steps to reproduce

  1. carthage bootstrap
  2. open demo app
  3. select "multiple letterbox view" demo
  4. click on one of the buttons to attach the controller to one of the letterbox views

When the controller is detached from the letterbox view, things seem to be ok (see commented code in sample)

defagos commented 7 years ago

The expected behavior is already documented. But, as your example proves, this behavior is both:

We cannot have more than one video layer per controller, this means that at most one Letterbox view can display the content of a controller. Therefore:

This should be not too much work, I'll see what I can do.

Thanks for your input!

defagos commented 7 years ago

In the meantime, if you want to reattach to a Letterbox view A a controller C initially associated with this view A, then attached to another Letterbox view B (without detaching it from A), you should nil out the controller property on A before assigning it again to C.

Translated in your code example, and performed for all Letterbox views you have, the following will work in a predictable way:

@IBAction func showVideoInFirstLetterboxView(_ sender: Any) {
    firstLetterboxView?.controller = nil
    firstLetterboxView?.controller = playerController
}

@IBAction func showVideoInSecondLetterboxView(_ sender: Any) {
    secondLetterboxView?.controller = nil
    secondLetterboxView?.controller = playerController
}

@IBAction func showVideoInThirdLetterboxView(_ sender: Any) {
    thirdLetterboxView?.controller = nil
    thirdLetterboxView?.controller = playerController
}
vpdn commented 7 years ago

Just found out that the mentioned workaround causes a bit of a glitch. When the controller is set to nil, the dismissing letterbox view will show a "no file" message, which looks to the user like an error.

Workaround would be to hide the player before nil'ing the controller, but that seems like another layer of workaround. 🤔 I think it would be best to fix the bug for good.

simulator screen shot - iphone 7 - 2017-11-22 at 11 28 08

vpdn commented 6 years ago

It would be really nice if there was an option to keep the last frame visible, when the controller is detached from the view (instead of showing "no file" as of now or blanking out the screen with a black view as planned). That would make transitions easier to do.

We could maybe keep a still ourselves by rendering the current playerview before detaching, however I'm not even sure that's possible since AVPlayer doesn't render on the CPU so UIView:snapshotView and UIView:drawHierarchy might not even render the view.

Our usecase: We show autoplayable videos in a feed. There's only exactly one video autoplaying at every moment but sometimes, there are two consecutive videos visible on screen. When the top video card is moving out of screen (cropped but still visible), the second one will need to start playing. Since both videos are still visible, we need some way to fade out the upper video and fade in and start the bottom video. To do so with a single letterbox controller cause some state management, since the controller can only be detached once the upper video has been faded out completely. Detaching in the completion block of the animation however can cause some race conditions when checking whether the player has a controller (will only be false after the animation). Currently, we have a single LB controller for each video cell, but that turns out to be quite CPU & memory intensive. We'd like to move to a single shared auto play controller instead.

defagos commented 6 years ago

We already implemented a small PoC for a Letterbox view without loaded player (only displaying and refreshing metadata), which would probably be a step in the right direction I think. But since we had no real demand for it at the time, we dropped it.

Thanks for your clear explanations, we'll try to think about what we could do.