Should unload method set src to empty string?

[JobaDiniz]

Is it ok to set the video src attribute to an empty string?
When I call unload method, html5 video player throws an error: MEDIA_ERR_SRC_NOT_SUPPORTED

This line of code sets src to empty string, but according to W3 this shouldn't be done:

The attribute, if present, must contain a valid non-empty URL potentially surrounded by spaces - attr-media-src

A string is a valid non-empty URL potentially surrounded by spaces if, after stripping leading and trailing whitespace from it, it is a valid non-empty URL - valid-non-empty-url-potentially-surrounded-by-spaces

A string is a valid non-empty URL if it is a valid URL but it is not the empty string. - valid-non-empty-url

[tdrews]

Good catch and thanks for the report. We'll take a look.

[joeyparrish]

@JobaDiniz , what browser and version are you using when it throws that error? I'm unable to reproduce in Chrome 43.

[joeyparrish]

Still don't have repro, but FWIW, I tried these ways as alternatives:

delete video.src; // has no effect.

video.src = null; // cast to a string, causes src to become a relative url to "null"

video.removeAttribute('src'); // makes src attribute a blank string, but does not unload the original source

[JobaDiniz]

Browser: Chrome Version 42.0.2311.152 m
Sorry... in the upcoming weeks I'm not available to help...
I never said this is a bug, that's why I posed the question "Should unload method set src to empty string?". It seems that everybody sets src to an empty string (searching on google how to unload html5 video).
I know how is hard trying to fix something that for you just works. I need to create a repro somehow (don't know how yet) when I have the time to do it.

[joeyparrish]

Ah, I see. How frequently does this error occur? What OS are you on?

[JobaDiniz]

OS: windows 8.1
On our environment, this always happened when we changed the current video.
In order to change video, we call shaka load method (which will call unload internally). We found out that is the unload method because we setup a call to unload when user closes our player and that raised MEDIA_ERR_SRC_NOT_SUPPORTED. Our service is telenor mytv. Near future there will be free content and you will be able to test what I'm saying.

[donato]

I am seeing this same error every time when calling unload()

Error thrown from : https://github.com/google/shaka-player/blob/master/lib/player/player.js#L231
Stream : http://samplescdn.origin.mediaservices.windows.net/e0e820ec-f6a2-4ea2-afe3-1eed4e06ab2c/AzureMediaServices_Overview.ism/manifest(format=mpd-time-csf)
OS : Mac OSX
Chrome : Version 43.0.2357.124 (64-bit)

[joeyparrish]

@donato, are you getting a MEDIA_ERR_SRC_NOT_SUPPORTED error like @JobaDiniz?

[donato]

Yes. There are two versions of the error. I get this one when step debugging during _player.unload() :

{
  MediaError
    code: 4
    __proto__: MediaError
        MEDIA_ERR_ABORTED: 1
        MEDIA_ERR_DECODE: 3
        MEDIA_ERR_NETWORK: 2
        MEDIA_ERR_SRC_NOT_SUPPORTED: 4
}

And this one when following the same steps without debugging

Uncaught (in promise) DOMException: The existing MediaKeys object cannot be removed while a media resource is loaded.

[joeyparrish]

I'm testing right now with Chrome 43.0.2357.65 on OS X 10.10.3. I'm using the latest Shaka Player test app to load the manifest @donato mentioned. I load the video, let it play for a few seconds, then call app.player_.unload() from the JavaScript console.

I'm unable to reproduce the problem. I've tried in both compiled & uncompiled modes.

@donato, are you seeing the error in the Shaka Player test app? Or only in your own application? Are there other steps involved in repro than just loading a source and then calling unload a few seconds later?

[JobaDiniz]

Hi @joeyparrish, I was able to reproduce the error in http://shaka-player-demo.appspot.com/. Before calling app.player_.unload(), run the following code:

var video = document.getElementById('video');
video.addEventListener('error', function(e){console.error(e)});

After calling app.player_.unload(), you should get

Event {}bubbles: falsecancelBubble: falsecancelable: truecurrentTarget: nulldefaultPrevented: falseeventPhase: 0path: Array[6]returnValue: truesrcElement: video#videotarget: video#videotimeStamp: 1434057371604type: "error"__proto__: Event(anonymous function) @ VM645:3

[joeyparrish]

@JobaDiniz, what's the value of video.error.code when this happens?

[JobaDiniz]

4

[joeyparrish]

@JobaDiniz, @donato, I finally see what's going on.

This error is fired on video, but after Player has stopped listening for events. this.eventManager_.removeAll() runs before the source is unloaded, so the Player does not receive this event.

As far as I can tell, this event is a completely harmless side-effect of the way we unload. Until we (not we=Google, but we=the internet) come up with a better way of unloading a video source to reuse the video tag, my recommendation is not to listen for errors on the video tag directly. Instead, listen for errors on Player.

Listening on the Player will filter out errors you shouldn't have to care about, like this one, and it will allow you to receive lots of important errors that aren't dispatched through the video tag, like DASH-related or network-related errors.

So this brings us back to the original topic: is it correct to unload this way?

My feeling on the matter is that yes, it is okay to unload this way, but I am very open to alternatives. As mentioned in an earlier comment, I tried several alternatives already, to no avail:

delete video.src; // has no effect.
video.src = null; // cast to a string, causes src to become a relative url to "null"
video.removeAttribute('src'); // makes src attribute a blank string, but does not unload the original source

If anyone has any other ideas, or if anyone can find a more canonically correct approach to unloading in a w3c doc somewhere, I would be happy to try it.

[JobaDiniz]

So how is the correct approach to listen for errors? Is there a code
snippet you can share?
On qui, 11 de jun de 2015 at 20:49 Joey Parrish notifications@github.com
wrote:

@JobaDiniz https://github.com/JobaDiniz, @donato
https://github.com/donato, I finally see what's going on.

This error is fired on video, but after Player has stopped listening for
events. this.eventManager_.removeAll() runs before the source is unloaded,
so the Player does not receive this event.

As far as I can tell, this event is a completely harmless side-effect of
the way we unload. Until we (not we=Google, but we=the internet) come up
with a better way of unloading a video source to reuse the video tag, my
recommendation is not to listen for errors on the video tag directly.
Instead, listen for errors on Player.

Listening on the Player will filter out errors you shouldn't have to care
about, like this one, and it will allow you to receive lots of important
errors that aren't dispatched through the video tag, like DASH-related or
network-related errors.

So this brings us back to the original topic: is it correct to unload this
way?

My feeling on the matter is that yes, it is okay to unload this way, but I
am very open to alternatives. As mentioned in an earlier comment
#80 (comment),
I tried several alternatives already, to no avail:

delete video.src; // has no effect.
video.src = null; // cast to a string, causes src to become a relative url to "null"
video.removeAttribute('src'); // makes src attribute a blank string, but does not unload the original source

If anyone has any other ideas, or if anyone can find a more canonically
correct approach to unloading in a w3c doc somewhere, I would be happy to
try it.

—
Reply to this email directly or view it on GitHub
#80 (comment).

[donato]

Listening to shaka player for errors is demonstrated here

      player.addEventListener('error', function(event) {
        console.error(event);
      });

[joeyparrish]

It's very simple, and there's an example in app.js:

player.addEventListener('error', function(event) {
  console.error(event);  // or whatever else you need to do with it
});

This pattern can be found in the test app and the samples in the tutorial.

Would it be helpful to call this out more explicitly in the docs somewhere?

[JobaDiniz]

I think it would.

Thanks for the snippets.

[joeyparrish]

I'm going to close this for now. I've filed an issue to update the docs as #106.