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

[BUG]: SoundRecorderUIState mysteriously becoming null #764

Closed
jfkominsky opened this issue Sep 9, 2021 · 5 comments
Closed

[BUG]: SoundRecorderUIState mysteriously becoming null #764

jfkominsky opened this issue Sep 9, 2021 · 5 comments

Comments

@jfkominsky
Copy link
Contributor

jfkominsky commented Sep 9, 2021

Filing this not because I expect anyone else to fix it but because I want to have a record of it so I'm sure I'm not hallucinating.

Basically there's a lot of failure states that arise in a RecordPlaybackController because it reads a SoundRecorderUIState object as null when it really shouldn't, and I have absolutely no idea how or why.

Quick overview: In order for a RecordPlaybackController to coordinate between a SoundRecorderUI and a SoundPlayerUI, it has the following internal variables:

  SoundRecorderUIState? _recorderState; 
  SoundPlayerUIState? _playerState;

These are assigned during the initialization of the SoundPlayerUI and SoundRecorderUI using the functions registerPlayer() and registerRecorder, which goes in two steps. First, there's an interface function that the UI widget actually calls:

void registerRecorder(BuildContext context, SoundRecorderUIState recorder) {
  RecorderPlaybackController.of(context)?._state.registerRecorder(recorder);
}

Then there's the function that is part of the RecordPlaybackControllerState _state that does the actual work (note: I am going to rename this stupid thing _registerRecorder because come on)

void registerRecorder(SoundRecorderUIState recorderState) {
    _recorderState = recorderState;

    // wire our local stream to take events from the recording.
    _recorderState!.dispositionStream!.listen(
        (recorderDisposition) => _localController.add(PlaybackDisposition(
            // TODO ? PlaybackDispositionState.recording,
            position: Duration.zero,
            duration: recorderDisposition.duration)));
  }

After this point, _recorderState should have a value, and there is no way I can see for it to be null.

The first time you access a page that contains this widget (e.g., in the example app), this all seems to work perfectly as intended, and you can access _recorderState however you want.

However, if you leave the page and then come back to it later, without restarting the app, something weird happens. registerRecorder is still called, and a non-null state is registered. I've managed to establish that using logging. Then we come to the stuff I added, which makes the player and recorder accessible or not depending on whether the other one is active, and something utterly bizarre happens. Here's the code I'm currently debugging with:

As part of RecordPlaybackControllerState :

void _onRecorderNew() {
    /// For the specific case of there already being something recorded and
    /// recording again, the play button needs to be removed.
    if (_playerState != null) {
      _playerState!.playbackEnabled(enabled: false);
    }
    if (_recorderState == null){ // todo it STARTS null? HOW?! Furthermore, it is executing registerrecorder successfully! 
      print("It starts off null!");
    }
  }

The interface function called by the SoundRecorderUI:

void onRecordingNew(BuildContext context) {
  print(context.toString());
  RecorderPlaybackController.of(context)!._state._onRecorderNew();
}

What happens is that it recognizes there is a valid context, it even tells me the SoundRecorderUIState's identity as part of that context, but then it says that _recorderState is null. And again, this only happens the second time you load the page with the widget on it.

I have replicated this behavior in another app as well. I am not sure how _recorderState is getting lost, or if it's accessing the wrong instance of RecordPlaybackControllerState somehow. I'm open to ideas. As an interim measure I can make things more null-tolerant, but I'd like to know where this stupid null is coming from in the first place because that's going to diminish functionality.

Also, doesn't happen for the SoundPlayerUIState at all, just the recorder. The SoundPlayerUIState works as intended throughout.

@Larpoux
Copy link
Collaborator

Larpoux commented Sep 9, 2021

Maybe I can answer some of these points.

1- A very long time ago (V3 or V4), before I started to work on this project, FlutterSoundPlayer sent a Null to the Stream used for the onProgress events, when the playback was finished.
I am convinced that it was a bad design and I replaced that by a callback "whenFinished".

2- The problem with the state of the recorder (and of the player, too) is that there are 3 people :

  • The App (or the Widget UI)
  • The FlutterSoundRecorder object
  • The tau-core library which talk directly to the OS

The Only guy among those 3 entities which really knows what is the real recorder state is the third one (the tau-core library).
I try to keep a valid state in the second one (the FlutterSoundRecorder object), but I cannot guarantee that the state in this object is always uptodate.

For the Player, all the calls to the tau-core library return the new-state of the Player.
This way, the FlutterSoundPlayer has probably a pretty good uptodate state.
FlutterSound Player implements also a verb "getStatus()" which call the tau-core library if it wants to be really sure of the state. This verb is asynchronous, and the App must await the result. This is not very convenient, and probably the App can consider that the state inside the FlutterSoundPlayer is correct and does not need to call "getStatus"

For the recorder, I did not implemented this mechanism : all the calls to tau-core return a void.
Actually, I am not sure that it is necessary to change that. My experience shows that the state which is guessed by the FlutterSoundRecorder object is correct.

3- The Recorder state inside the FlutterSoundRecorder is :

RecorderState _recorderState = RecorderState.isStopped;

It cannot be null

@jfkominsky
Copy link
Contributor Author

This is helpful to know but there are still a few puzzles.

The thing that is coming up null is not actually a RecorderState, it's a SoundRecorderUIState, in other words, the UI widget's state. I know that state is correctly initialized the second time I go to the page, I even know that it is registered, but for some reason, it's tripping null-checks when I try to do anything with it.

It feels like something is trying to run in the wrong context somehow. It may have more to do with how states are managed in general than with FlutterSoundRecorder itself, but I don't know. That there is this difference between the underlying FlutterSoundPlayer and FlutterSoundRecorder might have something to do with it.

Oh, and the icing on the cake is that the recording-and-playback still works. It will record correctly and then play back whatever you just recorded. It just does so while claiming that the widget UI doesn't exist.

@Larpoux
Copy link
Collaborator

Larpoux commented Sep 9, 2021

Each FlutterSoundPlayer and each FlutterSoundRecorder have a corresponding object in tau-core.
Each time the user open a new recorder/player, a new object is created inside tau-core.
Each time the user close a session, the corresponding object inside tau-core is released.
Each verbs called by the App involve the corresponding object in tau-core

This parallelism seems to work well and I never had any issue in this area.
Of course it is still possible that there are bugs here.
But I think that there is a good chance that the issues are in the Widget UI :
this code has been coded by someone who has very complicated ideas, and actually nobody really master this code.

@jfkominsky
Copy link
Contributor Author

Yes, I agree, it is in the widget UI somewhere. I will be making a PR tomorrow that will either have a real fix or a band-aid that stops it from generating crashes as a result of this weirdness.

@jfkominsky
Copy link
Contributor Author

Hunted down where things were going wrong and figured out a fix. I don't completely understand it but I think it has something to do with the fact that the Controller is an InheritedWidget and there's some issue with how of(context) works that is sometimes getting stuck on the wrong thing. PR #765 addresses the problem and adds some extra safety.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants