seel-channel / video_viewer

FLUTTER PACKAGE: Multiplatform minimalist video viewer with spectacular user experience.
https://pub.dev/packages/video_viewer
MIT License
56 stars 57 forks source link

Null check operator used on a null value (iOS) #23

Closed kyryloz closed 3 years ago

kyryloz commented 3 years ago

First of all, thank you for all your hard work.

As I see, this package also utilizes helpers library, which you also developed. When changing the orientation of the device during playback I randomly get the next error (more often after changing from landscape back to portrait):

Null check operator used on a null value

When the exception was thrown, this was the stack: 
#0      GlobalKeyHelperExtension.size (package:helpers/helpers/extensions/global_key.dart:33:40)
#1      _SwipeTransitionState._changeData.<anonymous closure> (package:helpers/helpers/transition.dart:206:40)
#2      Misc.onLayoutRendered.<anonymous closure> (package:helpers/helpers/misc.dart:11:66)
#3      SchedulerBinding._invokeFrameCallback (package:flutter/src/scheduler/binding.dart:1144:15)
#4      SchedulerBinding.handleDrawFrame (package:flutter/src/scheduler/binding.dart:1090:9)

As I see helpers uses a lot of ! operators. For example,

static void onLayoutRendered(void Function() callback) {
    WidgetsBinding.instance!.addPostFrameCallback((d) => callback());
}

or

Size? get size => this.currentContext!.size;

Using an exclamation mark operator is discouraged, since it's not safe, especially when accessing flutter BuildContext.

Anyway, I wanted to fix this issue somehow, but unfortunately, the stack trace leads me nowhere. Maybe you will be able to help identify which code is executed during orientation change?

My code is as simple as:

@override
  Widget build(BuildContext context) {
    return Center(
      child: VideoViewer(
        source: {
          '1080p': VideoSource(
              video: VideoPlayerController.network(url),
              subtitle: captions
                  .asMap()
                  .map((key, value) => MapEntry(value.label, VideoViewerSubtitle.network(value.url))))
        },
        controller: controller,
        language: VideoViewerLanguage.en,
        autoPlay: true,
      ),
    );
 }

I was trying to utilize onFullscreenFixLandscape property (btw, when exit the fullscreen it looks like it does not set the orientation back to portrait) and also just manual changing of the device orientation. All is resulting in the overseen issue.

Thank you in advance.

kyryloz commented 3 years ago

OK, I think I've found a place where this issue arises. It's in helpers package, transition.dart file, _SwipeTransitionState class.

The _changeData() method currently looks like this:

void _changeData() {
    Misc.onLayoutRendered(() {
      final Size? size = _containerKey.size;
      if (_swipeDirection != widget.direction || _size != size)
        setState(() {
          _size = size;
          _swipeDirection = widget.direction;
          switch (widget.direction) {
            case SwipeDirection.fromTop:
              _direction = Offset(0.0, -_size!.height);
              break;
            case SwipeDirection.fromLeft:
              _direction = Offset(-_size!.width, 0.0);
              break;
            case SwipeDirection.fromRight:
              _direction = Offset(_size!.width, 0.0);
              break;
            case SwipeDirection.fromBottom:
              _direction = Offset(0.0, _size!.height);
              break;
          }
          _tweenKey.state!.changeTween = _createTween();
        });
    });
  }

I've changed it to the following:

  void _changeData() {
    Misc.onLayoutRendered(() {
      final Size? size = _containerKey.currentContext?.size;
      if (size != null && (_swipeDirection != widget.direction || _size != size))
        setState(() {
          _size = size;
          _swipeDirection = widget.direction;
          switch (widget.direction) {
            case SwipeDirection.fromTop:
              _direction = Offset(0.0, -size.height);
              break;
            case SwipeDirection.fromLeft:
              _direction = Offset(-size.width, 0.0);
              break;
            case SwipeDirection.fromRight:
              _direction = Offset(size.width, 0.0);
              break;
            case SwipeDirection.fromBottom:
              _direction = Offset(0.0, size.height);
              break;
          }
          _tweenKey.state!.changeTween = _createTween();
        });
    });
  }

It fixes the issue. Unfortunately, it's in helpers package, so to fix the video_viewer helpers should be updated first.

Also, it's just the tip of the iceberg, to make these libraries more robust and not error-prone it's advised to properly migrate to null-safety, not just ignore the compiler warnings with ! operator.

seel-channel commented 3 years ago

THANKS A LOT! I always have had that null check operator error and unfortunately I couldn't solve it, but with your pull request the bug solved

kyryloz commented 3 years ago

Cool, I'm glad it helped. :)

Looking forward to the new version of the video_player. Unfortunately, this change in helpers means, that consumers should update their code since some values which were not nullable before are nullable now, but it's inevitable.

RounakTadvi commented 3 years ago

When will the next version be released?

seel-channel commented 3 years ago

I just updated helpers and video_viewer dependencies. I think this issue is solved