cuechange event not being triggered on audio only HLS streams

[ghost]

nomayonnaise commented on Sep 12, 2017, 10:04 AM UTC:

Description

The cuechange eventListener is not being triggered for audio only HLS streams when using this plug-in. The audio only stream contains 3 10 second .ts segments with AAC audio 64kb/s. This happens with a real live HLS stream as well.

Here are 2 examples with 3 ts segments

With audio only - cuechange event is not triggered
http://jsfiddle.net/8f05vyb2/

With video and audio - cuechange event is triggered
http://jsfiddle.net/p228339u/4/

Sources

With audio only
http://li720-23.members.linode.com/hls/audio.m3u8
With video and audio
http://li720-23.members.linode.com/hls/video.m3u8

Steps to reproduce

Explain in detail the exact steps necessary to reproduce the issue.

1. Open audio only link in Chrome
2. Play video (should autoplay)

Results

Expected

cuechange triggered, input value is updated

Error output

There is no error output, cuechange event not triggered.

Additional Information

This only happens with audio only .ts segments. .ts segments with video and audio work correctly. This example only has 3 10 seconds .ts segments so the player will stop after 30 seconds.

videojs-contrib-hls version

what version of videojs-contrib-hls does this occur with?
videojs-contrib-hls 5.10.1

videojs version

what version of videojs does this occur with?
video.js 6.2.8

Browsers

what browsers are affected? please include browser and version for each

Google Chrome 60.0.3112.113

Platforms

what platforms are affected? please include operating system and version or device and version for each

Windows 7,8.1,10, OSX/MacOs

Other Plugins

are any other videojs plugins being used on the page? If so, please list them with version below.

None

Other JavaScript

are you using any other javascript libraries or frameworks on the page? if so please list them below.

None

This issue was moved by forbesjo from videojs/videojs-contrib-hls#1255.

[ghost]

mjneil commented on Sep 12, 2017, 6:42 PM UTC:

Looks like metadata cues are not being added to the track in an audio only case. Comment in contrib-media-sources
https://github.com/videojs/videojs-contrib-media-sources/blob/master/src/virtual-source-buffer.js#L516
// TODO: are video tracks the only ones with text tracks? I guess this answers that question

[ghost]

nomayonnaise commented on Sep 13, 2017, 9:16 AM UTC:

Thanks mjneil for pointing me in the right direction.

[ghost]

jamesjieye commented on Mar 24, 2018, 3:41 PM UTC:

mjneil, I've created a pull request on videojs-contrib-media-source which fixes this issue. Could you merge the PR?

Thanks,
James

[ghost]

uraspaz88 commented on Apr 20, 2018, 1:05 PM UTC:

I don't think the merge has been done yet but I can confirm that the issue is still present. I'm using videojs-6.8.0, and videojs-contrib-hls-5.14.1. Something I noticed in my cross-browser/device testing was that Safari on OSX/iOS does fire the cuechange event with audio-only HLS streams, possibly due to Apple's native HLS support, but other browsers/devices fail to fire the cuechange event for audio-only HLS streams.

Related, the native HLS support built into Edge and Android seems to have an issue where the addtrack event doesn't fire when the initial ID3 info is received. In this case, using the hls.overrideNative option is a decent work-around. I'm a github noob, so if I can provide other info, let me know.

[ghost]

jamesjieye commented on Apr 20, 2018, 1:10 PM UTC:

uraspaz88, the pull request is still open videojs/videojs-contrib-media-sources#172.

The current workaround is to change the library in node_modules directly which is not ideal.

I've sent request to mjneil but no reply yet.

[ldayananda]

Looks like we are avoiding adding ID3 cues from audio streams

https://github.com/videojs/http-streaming/blob/master/src/mse/virtual-source-buffer.js#L482

as mux.js already handles the parsing of AAC for ID3 tags:

https://github.com/videojs/mux.js/blob/9d549aeccb3fbe6a3399250b8e3457e32f9660bf/lib/aac/index.js#L73-L75

We might want to port over this PR from contrib-media-sources(after testing):

videojs/videojs-contrib-media-sources#179

[berrutti]

Hello @ldayananda and @mparisi76. I'm writing to you because we have a video player that uses Video.js as a rendering engine, and we are currently experiencing this exact same problem (Cue points are no triggering cuechange events on audio-only streams).

I followed this issue around and I can see that there's some merged code for it already (#334).
The changes were on a file called src/mse/virtual-source-buffer.js of the previous version of the library, which currently doesn't seem to exist, at least not with a similar structure.

I have prepared a testing environment for you:
cue-points-test.vercel.app

This page has links to audio-only and video+audio streams. Both streams have cue points that emit metadata every few seconds. In the video page, you can see on the console that the metadata is printed every 5 seconds. In the audio-only one you can see the metadata being printed but only on Safari, which uses its own implementation of HLS.

Do you think you can have a look?

The actual streams are here:

https://cue-points-test.vercel.app/video/master.m3u8
https://cue-points-test.vercel.app/audio/master.m3u8

Thanks a lot!

Matias

[video-archivist-bot]

Hey! We've detected some video files in a comment on this issue. If you'd like to permanently archive these videos and tie them to this project, a maintainer of the project can reply to this issue with the following commands:

• for https://cue-points-test.vercel.app/video/master.m3u8: say @video-archivist-bot save lja8qE
• for https://cue-points-test.vercel.app/audio/master.m3u8: say @video-archivist-bot save gjx4Yj

[mister-ben]

As things currently stand (code has changed a lot since the original issue), I think this is because this expects the videoTimestampOffset() to return null for audio only, so the audio timestamp is not used

http-streaming/src/segment-loader.js

Lines 1875 to 1877 in afb1ff7

	const timestampOffset = this.sourceUpdater_.videoTimestampOffset() === null ?
	this.sourceUpdater_.audioTimestampOffset() :
	this.sourceUpdater_.videoTimestampOffset();

However that value defaults to 0 rather than null.

http-streaming/src/source-updater.js

Lines 337 to 339 in afb1ff7

	// initial timestamp offset is 0
	this.audioTimestampOffset_ = 0;
	this.videoTimestampOffset_ = 0;

Setting that default to null works for @berrutti 's audio stream.

[berrutti]

Hi @mister-ben, thank you very much for your reply. I could create a PR with this change if you think it would be helpful. I briefly checked the repository and from my really limited understanding, videoTimestampOffset would never return null unless explicitly set by calling videoTimestampOffset(null) correct? Is it safe to assume that changing null to 0 in that ternary would fix this issue?
Thanks again for your help! 🙏