Html5 src behaves incorrectly

[Akkuma]

If one calls player.src() the current code for Html5 media will actually overwrite the src on the video rather than appropriately return src or return currentSrc

[heff]

Have you tried the latest version? This shouldn't be the case anymore. https://github.com/videojs/video.js/blob/v4.5.2/src/js/player.js#L1025

[Akkuma]

@heff Yes, we are running the latest. The problem isn't in that code, since it is doing the right thing. If you follow the rabbit hole, you wind up in techGet, which will call Html5.prototype.src that isn't doing the right thing.

[heff]

Oh damn...looks like I missed that in @dmlap's src update. The techs would need a setSrc() method to be consistent with the other API methods. And that would be called whenever src is set.

In the techs we should switch all props to use setProp() and getProp(), instead of what it is now, setProp() and prop().

[Akkuma]

My personal preference is to use a single getter/setter function for properties. My optimal version would use something like Object.defineProperty to make a transparent replacement for the video element api, but isn't useable if you want to support IE8 and lower, which I personally don't need. I wrote a quick HTML5Player wrapper, when we couldn't get VideoJS working due to this bug (unknown at the time), that looked like this:

['src', 'paused', 'muted', 'volume', 'poster', 'currentTime'].forEach(function (_) {
    HTML5Player.prototype[_] = function (val) {
        if (val !== undefined) this.video[_] = val;
        return this.video[_];
    };
});

At least to me this approach avoids almost all possible bugs, since they are all defined in a uniform way. If someone preferred two different methods that would be pretty simple too via:

['Src', 'Paused', 'Muted', 'Volume', 'Poster', 'CurrentTime'].forEach(function (_) {
    HTML5Player.prototype[_.toLowerCase()] = function (val) {
        return this.video[_];
    };
        HTML5Player.prototype['set'+_] = function (val) {
        this.video[_] = val;
    };
});

Downside is this incurs a slight performance penalty obviously.

[gkatsev]

In ES6, we're getting proxies, so, we could also just proxy videojs on top of the video element api for extra awesomeness.

[Akkuma]

ES6 proxies will be really nice, but they'll be even less compatible than defineProperty :cry:

[heff]

Yeah, either of those wold be cool, but I think we're still far from dropping IE8. There's still people concerned about the lack of IE6/7 support.

The reason for using separate setters and getters at the tech level is because we're providing combined setter/getter functions at the player API level, and it seemed less than optimal to be doing the argument check twice for every API property.

[heff]

Following up on this one, I think all that really needs to be done here is:

• Add a setSrc method to the Flash and HTML5 techs
• Make the player call techCall('setSrc') instead of techCall('src') like it does now

We may want some kind of fallback to 'src' since this change could break external techs.

[sprice]

Looks like this one is done. See setSrc in html5.js and flash.js

[gkatsev]

@sprice looks like we're still calling techCall_('src'...). Which would need to change to call this one complete.

[gkatsev]

Probably actually deprecate setting the src through techCall(src

[clkao]

however there's currently no setSrc entry point in the player api that calls the ones provided by techs, the src method is doing techCall_('setSource') for setter

[sumitshinde-84]

i would like to work on this issue , can you please assign it to me?

[c0rdial]

I would like to contribute this.

[MANASA-NUKALA]

i want to contribute to this can you guide me? how to contribute?