flowplayer / flowplayer-hlsjs

Flowplayer HLS.js plugin
MIT License
81 stars 35 forks source link

Stop() and Shutdown() in flowplayer does not stop hlsjs #57

Closed KallDrexx closed 8 years ago

KallDrexx commented 8 years ago

We have observed with our live video player that if we call flowplayer.stop(), flowplayer.unload(), or flowplayer.shutdown() then HLSJS is still active in the background downloading m3u8 playlists as well as ts segments forever.

We have fixed this ourselves by manually calling flowplayer.engine.hlsjs.stopLoad() after stop() but before shutdown(), but I imagine this should be something the plugin handles automatically.

phloxic commented 8 years ago

@KallDrexx - are you sure that you are applying unload or shutdown correctly? They need a splash setup to work, see https://flowplayer.org/docs/api.html#methods FWIW I cannot repro e.g. at http://demos.flowplayer.org/basics/long.html - after starting playback, I press q while hovering over the player to unload, and the Network panel immediately aborts loading more segments.

KallDrexx commented 8 years ago

We are not using splash setup because we need multiple simultaneous video players on the same page at one time. However, even though Flowplayer docs say that shutdown and unload are only valid with a splash setup there does not seem to be any way to destroy a flowplayer + hlsjs instance properly otherwise.

We need to be able to completely stop hlsjs without being in splash mode.

KallDrexx commented 8 years ago

For reference, the (typescript) code I'm using to start flowplayer is:

var container = ctx.element.find("[player-layout='video'] div");

            window.flowplayer(() => {
                // NOTE: These must be done in a window.flowplayer() call to make sure
                // they are the first event 

                // disable click on screen
                ctx.element.find(".fp-ui").on("click touchstart", function (ev) {
                    // but do not disable click on other UI elements
                    // i.e. confine to parent element with class="fp-ui"
                    if ($(ev.target).hasClass("fp-ui")) {
                        if (ctx.isInDebouncePeriod) {
                            ev.stopPropagation();
                            return;
                        }

                        if (ev.touches !== undefined) { // Only do this for finger taps, not for mouse clicks
                            if (ctx.flowPlayer !== null && ctx.flowPlayer !== undefined && ctx.flowPlayer.playing) {
                                ev.stopPropagation();
                                ctx.toggleVideoSelector();
                                ctx.startDebounce();
                                return;
                            }
                        }
                    }
                });
            });

            ctx.flowPlayer = window.flowplayer(container, {
                live: true,
                autoplay: true,
                key: "xxxxx",
                brand: { showOnOrigin: true },
                clip: {
                    hlsjs: { listeners: ["hlsError", "hlsLevelLoaded"] },
                    sources: [{
                        type: "application/x-mpegurl",
                        src: ctx.activeVideo.hlsUrl
                    }]
                }
            });

            ctx.flowPlayer.on("finish", function () {
                // This means we got to the end of the playlist, reload the player 
                ctx.flowPlayer.load();
            });

            ctx.flowPlayer.on("hlsError", function (e, api, data) {
                if (data.details === window.Hls.ErrorDetails.LEVEL_LOAD_TIMEOUT ||
                    data.Details === window.Hls.ErrorDetails.MANIFEST_LOAD_ERROR) {
                    // trigger player network error
                    api.trigger("error", [api, { code: 2 }]);
                }

                // TODO: Add level load error counting
            });

            ctx.flowPlayer.on("hlsLevelLoaded", function () {
                // TODO: Add reset for level load error count
            });

            ctx.flowPlayer.on("ready", function (e, api) {
            });

            ctx.flowPlayer.on("error", function (e, api, err) {
                clearInterval(ctx.flowPlayerRetryTimer);
                ctx.setAsOffline();
                ctx.clearActiveVideo(true);

                // Retry if network or 404 error
                if (err.code === 2 || err.code === 4) {
                    ctx.failedStallCheckCount = 0;

                    ctx.clearActiveVideo(true);
                    ctx.setAsOffline();
                    ctx.startHlsVideoChecker();
                }
            });

and the code I am using for stopping flowplayer is:

            if (ctx.flowPlayer !== null && ctx.flowPlayer !== undefined) {
                ctx.flowPlayer.stop();
                if (ctx.flowPlayer.engine.hlsjs !== undefined) {
                    ctx.flowPlayer.engine.hlsjs.stopLoad();
                }

                ctx.flowPlayer.shutdown();
                ctx.flowPlayer = null;
            }

This was reproducible by calling the clear code without the hlsjs manual stop.

phloxic commented 8 years ago

That's what stopLoad is there for - note however that this will not work in generic playback, there is no interface available to do so. Also shutdown will not work as expected, it can't clean up all events properly if you don't use a splash setup. unload() or shutdown() in a non-splash setup will just call stop(). You could file a feature request for splash w/o unloading other players in the core issue tracker.

KallDrexx commented 8 years ago

I guess the core reason I created this issue for is that I don't think it makes sense to me to force us to manually trigger stopLoad(). I would expect flowplayer.stop() to do this for me because if the user says stop they probably don't want to be constantly wasting the bandwidth of continuing to download HLS segments they aren't watching. If the user wanted segments to continue downloading they would have paused.

phloxic commented 8 years ago

What stop() does is: pause the player, and seek to 0. I am actually looking into a config knob which calls stopLoad on pause, and while doing that I ran into problems, especially with live streams, but also with VOD, on resume afterwards; when the required advance buffer was not present. If I find a way to make that safe, at least with the majority of streams, then something similar can be implemented for stop() as well, but it would be engine specific. Right now it does what the core player does/can do.

KallDrexx commented 8 years ago

That's fine, I just wanted to at least raise this issue. I would have done a PR but I didn't see a quick and easy way to execute something on a stop() call only, which makes sense as you said stop is just a pause vs seek. Right now my cleanup code works for my use case at least, and I'll raise a unload without splash issue on the core issue.

Thanks,

nnarhinen commented 8 years ago

@blacktrash how about having something like this in this engine:

flowplayer(function(api) {
  api.on('stop', function() {
    flowplayer.engine.hlsjs.stopLoad();
    api.one('resume', function() {
      flowplayer.engine.hlsjs.startLoad();
    });
  });
});
nnarhinen commented 8 years ago

(at least please try if that works as a snippet in a page)

phloxic commented 8 years ago

As I said, the last time I tried this was not entirely reliable, at least not without a beforeresume event - buffer may be flushed, and player tries to resume; it may also be tricky to find the correct argument to startLoad() in case of DVR. It would also mean an engine specific behavior (generic playback will not be affected, did not check flash hls yet) - which is ok, but, as mentioned, I'm looking more into something like a bufferWhilePaused boolean option which can be set to false, and then doing this on stop automatically.

phloxic commented 8 years ago

In other words: when trying to implement this for pause, I had at least to startLoad on beforeseek to make sure it works; stop implies a seek to 0 but (especially) the first segments may be purged, and there you have the issue.

nnarhinen commented 8 years ago

Actually generic playback in most cases (with byte range requests) is much wiser than HLS.js or Flashls. It only buffers parts needed. HLS.js and flashls seem to buffer to the end for shortish clip always. And of course this has to be engine specific (engine is in charge of buffering, not the core player).

nnarhinen commented 8 years ago

And I also think that calling stop() should imply "also stop buffering". And it is / should be the responsibility of the engine.

phloxic commented 8 years ago

I was thinking more about generic HLS playback, which hls.js emulates quite well with its default setting. I also don't see e.g. MP4 buffering stopping on stop. In which browsers does that happen?

phloxic commented 8 years ago

The candidate where this works cleanly is RTMP because it requires an absolutely minimal and confined buffer to restart playback - server provides that capability.

phloxic commented 8 years ago

For the record, the buffer blocker for VOD works only in Chrome, and only when the dev tools are open with cache disabled, and only for short videos or some videos (strangely enough). Under normal circumstances e.g. http://demos.flowplayer.org/basics/minimal.html immediately buffers the entire video (all caches purged to beginning of time before loading). Other than that expunging buffers is governed internally by the client (one other reason why UHD should be encoded in HEAVC or vp9/10 because the limit would be reached too quickly otherwise).

phloxic commented 8 years ago

@KallDrexx - this can now be enabled for the hlsjs engine by setting the bufferWhilePaused hlsjs option to false.

KallDrexx commented 8 years ago

Awesome much appreciated! I'll give this a whirl in the next few days

josh-sachs commented 7 years ago

I'm not seeing this work in Flowplayer 6.0.5 commercial.

I can see in flowplayer.hlsjs.js

case "pause":
     if (!hlsUpdatedConf.bufferWhilePaused) {
        hls.stopLoad();
      }
break;

hls.stopLoad() doesn't appear to stop player from grabbing new m3u8 and .ts segments.

It is weird really, if I manually invoke player.engine.hlsjs.stopLoad() while playing === true, the hls loading stops and we hit an hlsError.

if I pause flowplayer with player.pause()... it doesn't stop. If i invoke player.engine.hlsjs.stopLoad() while paused=== true, loading doesn't stop.

Its like you can only stop loading while the player is playing.

josh-sachs commented 7 years ago

actually it dawned on me to try something. Looks like any XHR request for the m3u8 manifest will cause the .ts segments to download as long as the flowplayer instance exists (doesn't matter paused or not).

I have a background task that checks for m3u8 manifest while paused so that if the live stream ends while the player is paused, it will transition to an offline state.

I'm not sure if this is a behavior of the hlsjs engine, or of the video tag in chrome.

phloxic commented 7 years ago

@josh-sachs stopLoad() works fine in paused state? Maybe your scenario (not clear for me) is something like this: http://demos.flowplayer.org/api/live-check.html which indeed checks the xhr requests.

josh-sachs commented 7 years ago

I haven't worked on this in a while - but will actually be resuming shortly. I can update then.