Fix remaining race condition in media playback #1254
Conversation
This commit is hopefully the final fix on Airsonic's side for airsonic#685. It also fixes airsonic#1160, which was caused by temporary workarounds introduced in airsonic#1080 while we were looking for a solution. The root cause of the issue is the fact that, when we go to the next track in an Airsonic play queue, we change the media source in the `ended` event. In MEJS, this translates as the following two things: * In Airsonic's 'ended' event, we change the media source (set the `src` attribute) and call the `load()` method, followed by the `play()` method. * The 'ended' event was also used internally by the MEJS player, and one of these internal uses called the `pause()` method (presumably in order to make sure that playback was stopped on some media renderers). Unfortunately, the order in which these events are called depends (in all modern browsers) on the order in which they are registered. In our case, the first one is registered inside the `<body>` tag, but the second one is registered with `$(document).ready(...)`. This means that the first event handler is called before the second. This means that, in some cases (when we're unlucky, hence the seemingly random nature of the bug), `pause()` is called after `load()` but before the media has finished loading. Apparently, this causes the `AbortError: The fetching process for the media resource was aborted by the user agent at the user's request.` message to appear (which indicates exactly what's described in the last paragraph), and the playback of the next song is aborted.
613181c to
40ef550
Compare
|
@fxthomas I built your branch and ran the resulting container on our "live" server, and I don't have the last song repeating anymore on my Chrome browser, whereas it was always repeating with the latest release. Thank you, it seems you fixed it! |
|
Thanks for testing! I assume [edit: you] didn't find other issues with playback (such as the original bug where the next song isn't played)? Since this is a core feature that had a lot of difficult to debug issues lately, I want to leave it open until this weekend for either myself or other testers to find issues. If nothing happens, I'll merge it. |
We did not observe any regression in playback behaviour : ) |
|
I did not notice any issues for a few days now, time to merge this! |
|
@muff1nman I was thinking we should probably backport this and make a 10.4.1 release. This affects a significant portion of people and the patch applies cleanly on top of the |
|
Or, a new major release :3 |
|
@fxthomas sure. TBH I was hoping that we could get a 10.5 released soon, but I think it'd be smarter to do a 10.4.1. Can you submit a PR for this to the 10.4 branch? We should probably also pull in the mariadb change as well. |
TL;DR: This commit is hopefully the final fix on Airsonic's side for #685. It removes temporary workarounds introduced in #1080, and as such fixes #1160.
The root cause of the issue is the fact that, when we go to the next track in an Airsonic play queue, we change the media source in the
endedevent, as described in more detail in mediaelement/mediaelement#2650.In MEJS, this translates as the following two things:
In Airsonic's
endedevent handler, we change the media source (set thesrcattribute) and call theload()method, followed by theplay()method.Another
endedevent handler was also used internally by the MEJS player, and one of these internal uses called thepause()method (presumably in order to make sure that playback was stopped on some media renderers).Unfortunately, the order in which these event handlers are called depends (in all modern browsers) on the order in which they are registered.
In our case, the first one is registered inside the
<body>tag, but the second one is registered with$(document).ready(...). This means that the first event handler is called before the second.This means that, in some cases (when we're unlucky, hence the seemingly random nature of the bug),
pause()is called afterload()but before the media has finished loading.Apparently, this causes the
AbortError: The fetching process for the media resource was aborted by the user agent at the user's request.message to appear (which indicates exactly what's described in the last paragraph), and the playback of the next song is aborted.