Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Indicator is still showing if audioOnly is true #998

Closed
waster opened this issue May 20, 2016 · 26 comments
Closed

Indicator is still showing if audioOnly is true #998

waster opened this issue May 20, 2016 · 26 comments
Labels

Comments

@waster
Copy link

waster commented May 20, 2016

Browser: FF 46
OS: Ubuntu 16.04
Clappr Version: 0.2.51

Here is the code I tested on http://cdn.clappr.io:

var playerElement = document.getElementById("player-wrapper");

var player = new Clappr.Player({
  source: 'http://clappr.io/highline.mp4',
  baseUrl: '/latest',
  poster: 'http://clappr.io/poster.png',
  height: 360,
  width: 640,
  audioOnly:true
});

player.attachTo(playerElement);

var variant_streams = {
  'test': [
    {
      source: 'http://<URL>/playlist.m3u8',
      mimeType: 'application/x-mpegURL'
    }
  ]
};

player.load(variant_streams['test']);
player.play();

Source is the URL to the variant playlist of HLS with ts-segments with audio only stream.

I see that play button indicator is still showing while playing.

@waster
Copy link
Author

waster commented May 20, 2016

I also tested with the following multiple streams (with and without 'audioOnly:true'):

var variant_streams = {
  'test': [
    {
      source: 'http://<ULR_VARIANT1>/playlist.m3u8',
      mimeType: 'application/x-mpegURL'
    },
    {
      source: 'http://<ULR_VARIANT2>/playlist.m3u8',
      mimeType: 'application/x-mpegURL'
    },
    {
      source: 'http://<ULR_MP3>/stream.mp3',
      mimeType: 'audio/mp3'
    }
  ]
};

In this case bubbles spinner is showing infinitely while stream is playing.

@leandromoreira
Copy link
Member

Hey @waster I tried this code bellow at cdn and it works just fine, what is the problem are you facing?

var playerElement = document.getElementById("player-wrapper");

var player = new Clappr.Player({
  source: 'http://clappr.io/highline.mp4',
  baseUrl: '/latest',
  poster: 'http://clappr.io/poster.png',
  height: 360,
  width: 640,
  audioOnly:true
});

player.attachTo(playerElement);

var variant_streams = {
  'test': [
    {
      source: 'http://www.streambox.fr/playlists/x36xhzz/x36xhzz.m3u8',
      mimeType: 'application/x-mpegURL'
    }
  ]
};

player.load(variant_streams['test']);
//player.play(); this will raise an error cause https://bugs.chromium.org/p/chromium/issues/detail?id=593273

@leandromoreira
Copy link
Member

And if you want to pass multiple sources you should use sources: ['hls', 'mp4'] but keep in mind that when you pass multiple sources they will be used as fallback (like video tag) not as different bitrate options.

@waster
Copy link
Author

waster commented May 20, 2016

Sure, I understand and use multiple sources exactly as fallback, please check this code:

var playerElement = document.getElementById("player-wrapper");

var player = new Clappr.Player({
  source: 'http://clappr.io/highline.mp4',
  baseUrl: '/latest',
  poster: 'http://clappr.io/poster.png',
  height: 360,
  width: 640,
  audioOnly:true
});

player.attachTo(playerElement);

var variant_streams = {
  'test': [
    {
      source: 'http://hlsserverdev.hostingradio.ru:8080/hls/11/320/out.m3u8',
      mimeType: 'application/x-mpegURL'
    }
  ]
};

player.load(variant_streams['test']);
// Play is important as I want to play by clicking link or there is another way?
player.play();

See screenshot:
issue

Also please check this sources:

var variant_streams = {
  'test': [
    {
      source: 'http://hlsserverdev.hostingradio.ru:8080/hls/11/320/out.m3u8',
      mimeType: 'application/x-mpegURL'
    },
    {
      source: 'http://ep128.hostingradio.ru:8030/ep128',
      mimeType: 'audio/mp3'
    }
  ]
};

See attached screenshot:
issue2

In both cases streams are playing seamlessly and fine.

@towerz
Copy link
Member

towerz commented May 23, 2016

Confirmed the bug, will push a fix soon.

@waster
Copy link
Author

waster commented May 23, 2016

Great! Thanks for the quick response.

@towerz
Copy link
Member

towerz commented May 23, 2016

dc569a1 fixes this issue, could you please test against it?

@waster
Copy link
Author

waster commented May 23, 2016

hm, I did the following to generate dist directory:

npm install
npm run build

All dist files were generated sucessfully, but now I have errors in the console while opening my test page that worked before:

TypeError: resource.split is not a function
 clappr.js:18705:24
TypeError: _this7.container is null
 clappr.js:11229:12

Did I forgot to do something?

@leandromoreira
Copy link
Member

@waster what version are you using of npm? also try to run npm run release

@waster
Copy link
Author

waster commented May 23, 2016

Here are versions were used to build. npm run release did not help too.

# node -v
v4.2.4

# npm -v
2.14.12

What version of npm is compatible to build clappr?

@leandromoreira
Copy link
Member

leandromoreira commented May 23, 2016

@waster I think it's not a problem related to clappr itself but some libraries that we use and their dependencies requires a higher version. I'm using:

leandro at Mac-8 in ~/
$ node -v
v6.2.0

leandro at Mac-8 in ~/
$ npm -v
3.8.9

@waster
Copy link
Author

waster commented May 23, 2016

Sure, will try with higher version.

@tjenkinson
Copy link
Contributor

@waster @leandromoreira it's a bug in the change I added I'm looking into it at the moment.

@leandromoreira
Copy link
Member

And you still can use npm start and you'll have access to a local release 😄

@waster
Copy link
Author

waster commented May 23, 2016

@tjenkinson no problem, will wait for fix...8)
@leandromoreira thanks for the hint!

@waster
Copy link
Author

waster commented May 23, 2016

I've built from sources again using npm with npm run release and with clappr.js it does not work if player.load() and then player.play() are calling:

ReferenceError: startPlay is not defined

@tjenkinson
Copy link
Contributor

Wow today is going so smoothly...
I blame @towerz for that one :P

Should be fixed with 0087b8c

@waster
Copy link
Author

waster commented May 23, 2016

It plays now, thanks. @towerz but unfortunately I'm still seeing the problems with indicator:

  1. WIth audioOnly: true there is no play indicator after loading the player with first init source. However play()and load() methods works fine with multiple or standalone m3u8(s). However loading bubbles are not still hiding with additional source with mimeType: 'audio/mp3'.
  2. Without audioOnly: true with multiple sources play or bubbles indicators are still showing.

@tjenkinson
Copy link
Contributor

@waster can you provide an example where it doesn't work I just tried and it seemed fine for me.

@waster
Copy link
Author

waster commented May 25, 2016

Sure, will try to prepare and example ASAP.

@waster
Copy link
Author

waster commented May 26, 2016

Ok, I prepared the example page with the latest generated clappr.min.js where you can see the problems:

  1. Play indicator is not showing although player was initialized with source.
  2. Click on the "Click to play" link and you'll see that bubbles are showing all the time although stream is playing fine.

As you can see I pass two sources one with application/x-mpegURL mimeType and other with audio/mp3 as fallback and enabled 'audioOnly: true'.

@tjenkinson
Copy link
Contributor

@waster I never had the issue with the loading indicator going away but there were a few other problems which are fixed in some pending pull requests. Once they're merged fingers crossed it's sorted!

@waster
Copy link
Author

waster commented May 26, 2016

@tjenkinson I hope it will be fixed with sources plugin, so did you check example page I provided?

  1. There is no play indicator even source was loaded initially.

test clappr latest - mozilla firefox_001

  1. Bubble indicator is not hidden after clicking on "Click to play".

test clappr latest - mozilla firefox_002

Did you see the same behavior on the test page?

@tjenkinson
Copy link
Contributor

I had the play issue but not the loading indicator issue. Everything was working fine for me running the same code on the clappr demo page with my fixes merged.

Please can you try with this build so we can check we're using the same versions?
https://rawgit.com/tjenkinson/24105d6b6f3d10ad2a4eb67c04868edb/raw/beda69f3a93ff5ee2412e59d1de0b1121b7f9904/clappr.js

@waster
Copy link
Author

waster commented May 26, 2016

I tried your version and also build the latest min.js and it seems that everything is fine with indicator now. Great! I used clappr version generated before your messages about sources plugin fixes.

@tjenkinson
Copy link
Contributor

Great! I'll close this then :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants