Avoid attempting to schedule an audio fade out in cases where the duration of an audio resource is not known or is non-numeric (Inf/NaN)

[Fyorl]

Originally in GitLab by @Fyorl

Originally reported by yahs#3178 https://discord.com/channels/170995199584108546/848968607526551592/885268108477087754

ALL MODULES DISABLED? True
OS, Hosting, Browser: The Forge, Windows 11 Beta, Google Chrome
Short Description of Bug: Playlists with a fade out duration fade out immediately upon start but continue playing the current song. Adjusting volume fixes for the current track.
Simple steps to reproduce the bug: Set fade duration on a playlist (Tested at 500-2000ms) and press play on either a track or to play the playlist. Audio plays briefly before fading out and continuing the current track without any audio. Adjust volume, either globally or per track, and the audio returns for that track. Once the track has finished and the playlist moves on to the second, the problem repeats itself and can be corrected again.

[Fyorl]

Originally in GitLab by @aaclayton

Don't think I believe this one.

[Fyorl]

Originally in GitLab by @Fyorl

I was not able to reproduce it using the reported steps.

[Fyorl]

Originally in GitLab by @spaceflows

I am having the same issue. OS, Hosting, Browser: The Forge, Mac 11.5.2, Chrome. No console errors when this occurs.
I deleted the music files from Forge assets, re-uploaded, and still have the problem. Problem tracks (all of them, really) display this for elapsed time/total runtime: .

Elapsed time is right, but the total runtime is weird. Also, the tracks seem muted until I move the volume meter of the individual track or the Playlists on Global Volume.

[Fyorl]

Originally in GitLab by @Muzak_Fan

I can confirm I'm having the same issue. Windows 10 and Chrome via The Forge. I replicated the bug using the exact same steps as the OP independently during my game tonight. I think this is likely a problem with The Forge, as I moved over to that service just recently and did not have this issue when self-hosting.

[Fyorl]

Originally in GitLab by @kakaroto

I have done multiple tests and I can also confirm it's 100% reproducible for me as well. Do note that it only seems to happen when the duration is Infinity and you don't have the sound set to repeat. If you enable the Repeat option in the sound, the bug doesn't trigger.

As for why the sound shows a duration of Infinity, I've been debugging it yesterday and today and I can't figure out anything that would make it happen based on the server response. In the end, I found this https://www.thecodehubs.com/infinity-audio-video-duration-issue-fixed-using-javascript/ which indicates it's a chrome bug where the audio loads before the metadata has been read and the duration gets stuck on infinity.

I've tested their fix for it and can confirm it works.

Interestingly, in my tests, sometimes, Chrome would show the duration on its own (rarely) and sometimes it won't, but it always seemed that the onloadedmetadata was the event that triggered first, not onload, so I'm not sure how exactly this chrome bug/race condition is caused.

The fix would therefore be to change AudioContainer._createAudioElement from :

  async _createAudioElement() {
    console.debug(`${vtt} | Loading audio element - ${this.src}`);
    return new Promise((resolve, reject) => {
      const element = new Audio();
      element.autoplay = false;
      element.crossOrigin = "anonymous";
      element.onloadedmetadata = () => resolve(element);
      element.onload = () => resolve(element);
      element.onerror = reject;
      element.src = this.src;
    });
  }

into :

  async _createAudioElement() {
    console.debug(`${vtt} | Loading audio element - ${this.src}`);
    return new Promise((resolve, reject) => {
      const element = new Audio();
      element.autoplay = false;
      element.crossOrigin = "anonymous";
      element.onloadedmetadata = () => {
        // Workaround for Chrome bug which may not load the duration correctly
        if (element.duration == Infinity) {
          element.ontimeupdate = () => {
            element.ontimeupdate = undefined;
            element.currentTime = 0;
            resolve(element);
          }
          element.currentTime = 1e101;
        } else {
          resolve(element);
        }
      }
      element.onload = () => resolve(element);
      element.onerror = reject;
      element.src = this.src;
    });
  }

NOTE: The above fix is for the duration showing Infinity, it is not for the audio not playing until volume is adjusted when a fade out value is set. That bug still exists and I can provide you with music links where the issue is reproducible if needed.

[Fyorl]

Originally in GitLab by @aaclayton

Thank you for the additional information @kakaroto - we had some user feedback which made it difficult to disentangle whether this was specific to forge-hosted assets or not. I notice in your repro case you are also using an audio source from assets.forge-vtt.com. Did you perhaps reproduce this issue using a locally-hosted audio file? Or a remote-hosted file from a different source?

[Fyorl]

Originally in GitLab by @kakaroto

I wasn't able to reproduce the issue with a locally hosted file. When trying remote hosted files, they all failed due to CORS and using the cors-anywhere proxy caused it to work without issues for one of the remote files I tried. I couldn't figure out what the cause might be, or why it sometimes worked and sometimes didn't.

I tried to test and see if CORS headers are affecting it, but to no avail. With some additional testing today, I pinpointed the cause to possibly be due to Cloudflare returning a "200 OK" instead of a "206 Partial Content" for the Range request since the range is 0- (the full file), though I have also seen it working without issue when the response is a 200 instead of 206, so it's not even really consistent on when the chrome bug gets triggered... Anyway, that's where I got, likely putting the blame on the 200 vs 206 response status (even if inconsistent) as a probable trigger for the Chrome bug that shows Infinity as duration. I can't find anything else abnormal/different from requests on other hosts.

I'm sure the issue could happen with other remote hosts as well, so having the workaround in core for v9 would be a good idea, even if we fix it on Forge hosts client side with our module.

I hope that helps, let me know if you need any more info.
Thanks!

[Fyorl]

Originally in GitLab by @kakaroto

Can this be scoped for v10 by the way? I worry it might get forgotten as it has no milestone set.

Also, should I create a separate issue for the chrome bug+Cloudflare issue causing that "Infinity" duration (Fix posted in comment above) so it's not forgotten, or is it ok to just leave it with this issue (since the Infinity duration bug is not what this issue is about, but it's rather a prerequisite to triggering it).

Thanks.

[Fyorl]

Originally in GitLab by @aaclayton

Could you briefly summarize what the issue here is which is separate from the issue of the audio element having an invalid duration? I skimmed the issue but I think what I'm missing is a concise statement of what the problem is.

[Fyorl]

Originally in GitLab by @kakaroto

Sure. When the sound has a fade out value, then when it starts playing, no sound will be audible, even though it's still playing, adjusting the volume (or doing anything else that affects the playlist, I think) will cause the sound to start outputting audio.
It only seems to happen when there's a fade out value and I only reproduced it when I see the infinite duration bug too. It might just be that it starts fading out immediately when it starts playing, probably because it thinks it's time to fade the volume out already.

[Fyorl]

Originally in GitLab by @aaclayton

Could you explain why you think this bug is separate from the infinite duration issue if it only occurs in cases where the audio resource is incorrectly detected as having infinite duration? I'm not surprised that some timing-related features go wrong when the duration is non-finite. We might be able to harden that as a further precaution, but it all seems quite related to me.

[Fyorl]

Originally in GitLab by @kakaroto

They are related, for sure, but to me they are two separate bugs. Fixing the situation where the audio duration is infinite doesn't fix the rest of the logic which originally caused the sound not to play any audio, the infinite duration is only a trigger of it, and there might be other things that could trigger it as well. Or there might be a situation where an infinite duration is actually valid (i.e: playing from a live radio feed for example).

Adding a check on Infinite/NaN durations and ignoring the fade option in those cases could be the fix for this issue, while forcing the track to calculate its duration as a workaround for the Chrome/Cloudflare bug is a separate fix for a different issue, even if both are intimately related.

Hopefully that helps clarify why I viewed them as separate things.

[Fyorl]

Originally in GitLab by @kakaroto

Follow up on this. After testing the fix via the Forge's optional module, we've found that for some audio files, the ontimeupdate event is triggered, but for some other audio files (I'm guessing mp3 vs ogg), we never get a ontimeupdate signal, instead we get a ondurationchange event, and the element.currentTime gets set to 0 since it went above the duration limit.

Here's our current fix for it, which also includes a 5 second timeout, just in case there's a live source that's being used for which the duration will never change from Infinity, so we don't wait forever for those :

                    // After creating the element, if its duration was not calculated, force a time update by seeking to the end
                    if (element.duration != Infinity)  return element;
                    // Workaround for Chrome bug which may not load the duration correctly
                    return new Promise(resolve => {
                        // In case of a "live source" which would never have a duration, timeout after 5 seconds
                        const timeoutId = setTimeout(() => resolve(element), 5000);
                        // Some mp3 files will signal an `ontimeupdate`
                        element.ontimeupdate = () => {
                            element.ondurationchange = undefined;
                            element.ontimeupdate = undefined;
                            clearTimeout(timeoutId);
                            element.currentTime = 0;
                            resolve(element);
                        }
                        // Some ogg files will signal `ondurationchange` since that time can never be reached
                        element.ondurationchange = () => {
                            element.ondurationchange = undefined;
                            element.ontimeupdate = undefined;
                            clearTimeout(timeoutId);
                            element.currentTime = 0;
                            resolve(element);
                        }
                        element.currentTime = 1e101;
                    });

[Fyorl]

Originally in GitLab by @aaclayton

Wow... that's quite ugly 😞

We already merged in a "fix" for this, but I'll ping @Fyorl to take another look at it before Patch 2 ships.

[Fyorl]

Originally in GitLab by @kakaroto

Yeah, it is. It turns out that OGG files (or at least, the ones I was testing with) can't be played until the duration is found, so we see the track position increase, with duration Infinity but no audio can be heard (and the current time will go way beyond the real duration). And it also looks like it's unable to find the duration until it downloads the full audio. Interestingly, they all do signal ondurationchange so it looks like the ontimeupdate might not be necessary... but maybe some other sources will do the ontimeupdate signal but never do a ondurationchange (I'm mostly thinking a live podcast or something).

I sent the relevant info/links to fyorl, so hopefully they can duplicate and come to similar/better conclusions.

[Fyorl]

Originally in GitLab by @Fyorl

Yes, with the resources @kakaroto sent, I was able to perform a deeper investigation. However, my conclusion was that this is an issue of a misconfigured server rather than a browser bug.

For my tests I compared a local mp3 file, an mp3 and ogg file uploaded to S3, an mp3 and ogg file that @kakaroto provided hosted on the Forge CDN, and a radio source.

In each case, when the audio element's src is set, the browser makes a request with a Range: bytes=0- header. If the server responds with a 206 Partial Content with appropriate Content-Length and Content-Range headers, then the duration is immediately correct upon the first firing of the loadedmetadata event. If the server instead responds with a 200 OK with no Content-Length or Content-Range, then the duration is reported as Infinity, and the currentTime workaround is needed.

Requests to the Forge CDN do not respond with a Content-Length or Content-Range, they only supply a Content-Type, and respond with 200 rather than 206. Requests for a local file (i.e. served via Express) or to S3 respond with all three of Content-Type, Content-Length, and Content-Range, and respond with 206. Requests for a radio source respond in much the same way as the Forge CDN, i.e. only Content-Type and a 200 code. This leads me to believe that the Forge CDN is signalling via its response headers that the audio source is an infinite stream and the browser is therefore behaving as such.

I observed the same behaviour in both Firefox and Chrome, with the exception that Firefox seems to automatically fire an additional durationchange event after the loadedmetadata event for the two Forge-hosted files, possibly as a workaround for this common server misconfiguration.

[Fyorl]

Originally in GitLab by @Fyorl

There are some additional things I observed that we wanted to share in case they're relevant to your optional forge module:

I was able to verify that durationchange is all that's required to listen for, it fires reliably for both mp3 and ogg files with all server configurations. It does not fire for the radio source, but that does not fire a timeupdate either.

When setting the currentTime to a large value, in cases where the server does not correctly respond to a Range request, I believe this causes the entire file to be downloaded. I'm not certain, because it wasn't made particularly clear in the dev tools network tab, but that did appear to be what was happening. The durationchange will not fire until the whole file has downloaded, and any calls to play before then will not resolve until afterwards, meaning playback cannot be streamed at all.

Ogg files will not play at all even after fully downloading, but will throw a DOMException: The element has no supported sources. instead. Setting the currentTime to 0 does not resolve this, they have to be entirely unloaded. Infinite duration sources will, naturally, continue downloading infinitely and a call to play will never resolve.

Because of this, the setTimeout code to cancel the seek is not sufficient. For ogg files and infinite sources, correctly cancelling the seek would involve setting src to "" to unload the file and prevent the browser continuing the download, then it would require setting the src again but remembering to not re-attempt the infinite duration workaround again. I'm not even sure if there's a good way to record the previously ascertained duration of an ogg file because you can't set the duration field, it's read-only.

These additional complications mean we've opted to not try to include the proposed workaround in core code, but we have hardened all the audio code paths against the presence of infinite duration tracks meaning they should all play and function correctly with the minor exception that they might not fade out automatically at the end sometimes. The issue of immediate fade out for infinite duration tracks is resolved though, and consequently truly infinite streams like radio sources are also gracefully handled.

[Fyorl]

Originally in GitLab by @kakaroto

Hi Kim,

Thank you very much for the thorough investigation and report.
I can confirm that the entire file seems to be downloaded when we set currentTime to a high value, I've had a user report out of memory crashes because of that, which defeats the purpose of the streaming API for large music files. This makes the workaround unusable in my opinion.
I confirmed also that S3/our servers are responding with 206 partial response, and providing the range/content-length headers in the response, but Cloudflare doesn't seem to use it when the file isn't cached. At the moment, I'm quite unhappy with the overall result and the fix is not a good one either in my opinion, so I agree that it shouldn't be put into core at this time.

I've temporarily added a custom rule in Cloudflare to bypass the cache for ogg and mp3 files, which has fixed the duration problem immediately, and we'll be investigating further on what set of conditions causes CF to misbehave then we'll file a bug with them, hopefully they can get it fixed, or we'll find a better way to work around it.

Note that I didn't have the issue about "the element has no supported sources" or the need to reset the src, and I only noticed the bug on Chrome, as Firefox is able to determine the duration (though it takes a second as you noted).

Sorry about the headaches this issue brought you, but I'm glad to know that now the core is hardened against these types of misbehaving servers, and able to better handle live sources.

Thanks!