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

fix video crashing on Android because of unresolved ExoPlayer breaking changes #154

Merged
merged 3 commits into from
May 31, 2024

Conversation

Buthrakaur
Copy link
Contributor

This fixes video crashing on Android because of unresolved ExoPlayer breaking changes (mainly because of the new threading model https://developer.android.com/reference/androidx/media3/exoplayer/ExoPlayer#threading-model)
ReactVision/viro#229

@Buthrakaur
Copy link
Contributor Author

Hello @robertjcolley , could you please review this and possibly release new react-viro version if all is ok?

…is already the main thread - this fixes also problem with disposing the video instance when AR scene is unmounted as it causes `pause()` to be called and somehow deadlocks threads
… the AVPlayer instance (random ANR could be caused by the app waiting for the main thread to complete releasing the AVPlayer instance)
@doranteseduardo
Copy link
Member

doranteseduardo commented May 14, 2024

I am reviewing this PR
Edit: Unfortunately the crash persists, I will research further

@Buthrakaur
Copy link
Contributor Author

Hello @doranteseduardo ,
could you please share your repro steps and reopen the PR so we can fix any remaining issue and make the video work in viro? I tracked this crash down and fixed in viro-starter-kit (build viro-code => build viro => copy AAR artifacts to viro-starter-kit => build and run viro-starter-kit) and integrated as a patch into our own app and changes in this PR definitely fixed the crashing video for me. We are actually already using this in production - see the screen recording here: https://drive.google.com/file/d/1Bf959n_cfSL8rkDweHGVfqARj4NZxUch/view?usp=sharing

@Buthrakaur
Copy link
Contributor Author

  • my viro-starter-kit test code:
const HelloWorldSceneAR = () => {
  const [text, setText] = useState("Initializing AR...");
  const [videoStatus, setVideoStatus] = useState<
    "playing" | "paused" | "finished" | "not ready"
  >("not ready");
  const [videoPaused, setVideoPaused] = useState(false);
  const videoRef = useRef<ViroVideo>();

  function onInitialized(state: any, reason: ViroTrackingReason) {
    console.log("onInitialized", state, reason);
    if (state === ViroTrackingStateConstants.TRACKING_NORMAL) {
      setText("Hello World!");
    } else if (state === ViroTrackingStateConstants.TRACKING_UNAVAILABLE) {
      // Handle loss of tracking
    }
  }

  return (
    <ViroARScene onTrackingUpdated={onInitialized}>
      <ViroText
        text={text}
        scale={[0.5, 0.5, 0.5]}
        position={[0, 1, -1]}
        style={styles.helloWorldTextStyle}
      />
      <ViroVideo
        ref={videoRef}
        source={{ uri: 'https://www.w3schools.com/html/mov_bbb.mp4' }}
        position={[0, 0, -1]}
        scale={[0.8, 0.44, 0.5]}
        loop={false}
        paused={videoPaused}
        onBufferEnd={() => setVideoStatus("playing")}
        onFinish={() => setVideoStatus("finished")}
      />
      {videoStatus === 'playing' && (
        <ViroText
          text="Pause"
          position={[0, -0.75, -1]}
          onClick={() => {
            setVideoPaused(true);
            setVideoStatus('paused');
          }}
        />
      )}
      {videoStatus === 'paused' && (
        <ViroText
          text="Play"
          position={[0, -0.75, -1]}
          onClick={() => {
            setVideoPaused(false);
            setVideoStatus('playing');
          }}
        />
      )}
      {videoStatus === 'finished' && (
        <ViroText
          text="Replay"
          position={[0, -0.75, -1]}
          onClick={() => {
            videoRef.current?.seekToTime(0);
            setVideoPaused(false);
          }}
        />
      )}
    </ViroARScene>
  );
};

@Buthrakaur
Copy link
Contributor Author

Hello @doranteseduardo , any update? I spent quite some time with this PR so it's a bit demotivating to get the PR closed with no discussion or attempt to resolve any issues you discovered..

@doranteseduardo
Copy link
Member

@Buthrakaur I am sorry for the late response, I have been reviewing requests from the users on Discord.
I will get back to this later this week.

@doranteseduardo
Copy link
Member

Update:
I completely reviewed this PR and found that the issues I had before were not in here, but a bad build.
My apologies.

Thanks so much for bringing this fix to us :)

@doranteseduardo doranteseduardo self-requested a review May 31, 2024 18:14
Copy link
Member

@doranteseduardo doranteseduardo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this and works like a charm.

@doranteseduardo doranteseduardo merged commit c719c07 into ReactVision:main May 31, 2024
6 checks passed
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.

2 participants