E-Kuerschner / useAudioPlayer

React hooks for controlling audio on the web
MIT License
330 stars 36 forks source link

Maximum update depth exceeded when using React-Router #52

Closed jorgev259 closed 2 years ago

jorgev259 commented 3 years ago

Using the Provider and audio hooks under React-Router produces a Maximum update depth exceeded error loop

const routes = {
  '/': () => <Provider station='clouds' />,
  '/:station': ({ station }) => <Provider station={station} />
}

function Provider ({ station }) {
  return (
    <AudioPlayerProvider>
      <Radio station={station} />
    </AudioPlayerProvider>
  )
}

function Radio (props) {
  const [song, setSong] = useState({
    album: 'Press the Play button to start the radio',
    title: placeholders[Math.floor(Math.random() * placeholders.length)]
  })

  const { togglePlayPause, ready, loading, playing, volume, mute } = useAudioPlayer({
    src: `https://play.squid-radio.net/${props.station}?cache_ts=${new Date().getTime()}`,
    onend: () => console.log('sound has ended!')
  })

  return (
    <div />
)
}

const App = () => {
  const routeResult = useRoutes(routes)
  return routeResult || (
    <script>{(
      window.location.href = '/404.html'
    )}
    </script>
  )
}

Environment (please complete the following information):

E-Kuerschner commented 3 years ago

Thank you for the issue @jorgev259!

Before I dig too deep into this I noticed this line:

src: `https://play.squid-radio.net/${props.station}?cache_ts=${new Date().getTime()}`

The consequence of this, is whenever Radio re-renders a new url will be generated for the src of the hook (Because getTime() will return something new every call). I believe this will cause the state inside the AudioPlayerProvider to reset and in turn cause Radio to re-render once again which could the the origin of your stack overflow.

Is there a way you can have a stable src url? If not we may need to figure out another solution.

jorgev259 commented 3 years ago

Context: This is done to bypass the audio running a cached version, since its a Icecast stream.

Removed new Date().getTime() since it works without it but the depth issue remains.

E-Kuerschner commented 3 years ago

Alright thanks for checking that for me. I'll investigate it in the coming days and report back here.

jorgev259 commented 3 years ago

Tested making it totally static "https://play.squid-radio.net/persona" and the issue remains

jorgev259 commented 3 years ago
import React, {useState} from 'react'
// import { useRoutes } from 'hookrouter'

import { AudioPlayerProvider, useAudioPlayer } from "react-use-audio-player"

/* const routes = {
  '/': () => <Provider station='clouds' />,
  '/:station': ({ station }) => <Provider station={station} />
} */

function Provider ({ station }) {
  return (
    <AudioPlayerProvider>
      <Radio station={station} />
    </AudioPlayerProvider>
  )
}

function Radio (props) {
  const [song, setSong] = useState({
    album: 'Press the Play button to start the radio',
    title: 'TEST'
  })

  const { togglePlayPause, ready, loading, playing, volume, mute } = useAudioPlayer({
    src: 'https://play.squid-radio.net/persona',
    onend: () => console.log('sound has ended!')
  })

  return (
    <div />
  )
}

const App = () => {
  /* const routeResult = useRoutes(routes)
  return routeResult || (
    <script>{(
      window.location.href = '/404.html'
    )}
    </script>
  ) */

  return <Provider />
}

export default App

Even removing hookrouter still produces the same effect, no idea whats causing it then

E-Kuerschner commented 3 years ago

@jorgev259 i think it has something to do with the audio source https://play.squid-radio.net/persona. I was able to replicate your issue but when i switched to a static sound file which i was hosting from my local machine the problem went away. Something about playing sound from that URL causes the hook to keep loading it over and over again

jorgev259 commented 3 years ago

Does the library rely on the media being fully loaded? Might be that its never fully loading due to being a stream or its hitting the end multiple times. Any suggestions?

E-Kuerschner commented 3 years ago

@jorgev259 that is a good theory. I'm not very familiar with streaming audio, but I could see the possibility where that affects how the library behaves. Do you have another link from a different site which streams audio that I could try? I will do some more debugging this week

jorgev259 commented 3 years ago

https://stream.toohotradio.net/128

E-Kuerschner commented 3 years ago

hey @jorgev259 I think I'm on to something. I should hopefully have a release out by the weekend with a fix or work-around for the issue you are having with streaming audio

E-Kuerschner commented 3 years ago

@jorgev259 this should be address in 1.2.4 which is up now. Also please see the updated README, there is a section at the bottom regarding streaming audio. Cheers!

ybelenko commented 3 years ago

Got the same issue with version 1.2.4 I have a list of recorded phone calls(right now test sources below), so there are table with 10 rows and play button with progress in each row:

import React from 'react';
import { FormattedMessage } from 'react-intl';
import { useSelector } from 'react-redux';
import { useAudioPlayer, useAudioPosition } from 'react-use-audio-player';
import { Progress } from 'reactstrap';
import { selectById as selectCallLogById } from '../../redux/slices/callLogsSlice';

const PlayRecordingBtn = ({ callLogId }) => {
  const callLogData = useSelector((state) => selectCallLogById(state, callLogId));
  const { recordingUrl } = callLogData;
  const { togglePlayPause, playing } = useAudioPlayer({
    src: recordingUrl,
    format: 'mp3',
    html5: true,
    autoplay: false,
  });
  const { percentComplete } = useAudioPosition({ highRefreshRate: true });

  return (
    <div className="audio">
      {/* eslint-disable-next-line jsx-a11y/media-has-caption */}
      <button
        className={`togglePlay ${playing ? 'active' : ''}`}
        type="button"
        onClick={togglePlayPause}
      >
        Play/Pause
      </button>
      <Progress value={percentComplete} />
    </div>
  );
};

export default PlayRecordingBtn;

Also noticed warning in browser console:

HTML5 Audio pool exhausted, returning potentially locked audio object.

Audio sources that I used:

ybelenko commented 3 years ago

@E-Kuerschner should I submit new issue? I think I described it well above.

E-Kuerschner commented 3 years ago

@ybelenko I can reopen this issue, thanks for commenting.

When does the issue occur? I'm not sure i can recreate your experience with what you have provided me above. Your usage looks pretty typical in my experience. The only thing I can think of is maybe your selector hook is causing a bunch of unnecessary rerenders. The useAudioPlayer hook should be fine with that, but if its not then maybe that's a slightly different issue that needs to be addressed.

ybelenko commented 3 years ago

When does the issue occur?

It happens immediately on dev server build before any redux actions. I can check with vanilla Redux connect and avoid using selector if you struggling recreate my issue.

Maybe you should check to render long audio list with playing the same audio track?

E-Kuerschner commented 2 years ago

I recently discovered some code that could lead to an infinite loop and fixed it in version: https://www.npmjs.com/package/react-use-audio-player/v/1.2.5

Curious if this will alleviate any of the issues mentioned above