Live alt audio inspired on fredvb and pull 700 #832

sergiojm · 2016-11-20T17:48:40Z

Live alternative audio inspired on
fredvb pull request 727 and INIT_PTS_FOUND on pull request 700
using base code 0.6.10

merge

inspired on fredvb pull request 727 and INIT_PTS_FOUND on pull request 700 using 0.6.10 base code

mangui · 2016-11-21T17:53:03Z

thanks @sergiojm
as discussed on Slack, remuxing of alternate audio fragment should not start until INIT_PTS_FOUND event has been fired.
this is to maintain consistent timestamp synchronization between alt audio and main playlist and avoid AV sync issue.

one way to do it would be to add a new state in audio-stream-controller
WAITING_INIT_PTS name would do the trick

when a new alt audio frag is loaded, you should then check if initPTS is defined.

if defined, you could start remuxing as it is done today
if not defined, you should save fragment payload in class context, then switch to WAITING_INIT_PTS state

onInitPtsFound:
if WAITING_INIT_PTS state you should also trigger audio frag remuxing.

Changing BUFFER_FLUSH on audio with alternative audio On Live alt audio if we FLUSH Buffer imediatly can hold the play and delay that can lead to a moment that Alt audio is not available anymore Also now if it user pauses, gets away from live, switch audio track, would not break cause it will wait until it has a valid frag to flush

…spired_on_fredvb_and_pull_700

mangui

thanks @sergiojm I made a couple of comments, some are cosmetic, main point i am not clear about is all the stuff related to this.prevTrackId in audioStreamController. If you could explain in layman's term what you are trying to achieve it ?
thanks !

mangui · 2016-12-01T18:04:27Z

src/controller/audio-stream-controller.js

+
+            var currentTime=this.media.currentTime;
+
+            // When switching audio track the bufferEnd should only be considered


I am not clear about these code changes, could you explain what you are trying to achieve here ?

Se bellow comments

mangui · 2016-12-01T18:05:08Z

src/controller/audio-stream-controller.js

@@ -229,6 +271,22 @@ class AudioStreamController extends EventHandler {
            }
          }
          if(frag) {
+            //Only flush when we have a startPTS
+            //Calculating when we have a new track frag available to play


not clear as well why flushing is done before FRAG_LOADING, could you explain the logic here ?

When we flush(0 to infinity audio) before loading and parse the stream get stuck until we receive the track, if we ever recieve it.

On VOD no impact since playlists are always available on any current time.

On Live streams, especially with low chunk durations, you get error on playlists and the player just hangs the playback since it cannot download a playlist with chunk for the current time, just downloads forward chunks and get a hole on the buffer since we already flushed.

The ideia is, trigger that a audio switch occur, dont flush, and when the frag is loaded and parsed(with correct start), then flush and append to buffer. Meantime it just continues to play the current audio buffer until is safe to make the proper flush and append.

The problem increases if u change multiple times the audio track, cause every time u just get delay from the live pos(due to the flush 0 to inifity). Will lead u to a position when the playlist simple doesn't have any valid ts for the currentTime.

Also if u pause a live stream with alt audio, then resume and switch the audio track, the player will immediatly crash, cause you are forcing the behavour.

So now with flushing only when you download and parsed a valid frag for the currentTime you can even pause for long secondes cause it will use the current audio track in buffer until
its safe to flush and append.

mangui · 2016-12-01T18:09:54Z

src/controller/audio-stream-controller.js

+          let accurateTimeOffset = details.PTSKnown || !details.live;
+          this.demuxer.push(data.payload, audioCodec, null, start, fragCurrent.cc, trackId, sn, duration, fragCurrent.decryptdata, accurateTimeOffset, this.initPTS);
+        } else {
+          logger.log(`No video PTS for audio frag ${sn} of [${details.startSN} ,${details.endSN}],track ${trackId} , waiting for video pts`);


I would suggest renaming 'No video PTS' into 'unknown video PTS'

Yes good point will patch tomorrow

mangui · 2016-12-01T18:12:32Z

src/events.js

@@ -47,6 +47,8 @@ module.exports = {
  AUDIO_TRACK_LOADING: 'hlsAudioTrackLoading',
  // fired when an audio track loading  finishes - data: { details : levelDetails object, id : audio track id, stats : { trequest, tfirst, tload, mtime} }
  AUDIO_TRACK_LOADED: 'hlsAudioTrackLoaded',
+  // fired when the first timestamp is found.
+  INIT_PTS_FOUND: 'hlsInitPtsFound',


could you also describe the data object dispatched along with this event ?
you need to make it consistent with other events dispatched by demuxer:

{ id : demuxerId, initPTS: initPTS}

Yes good point will patch tomorrow

mangui · 2016-12-01T18:13:53Z

src/remux/mp4-remuxer.js

@@ -149,6 +153,7 @@ class MP4Remuxer {
      if (computePTSDTS) {
        initPTS = Math.min(initPTS,videoSamples[0].pts - pesTimeScale * timeOffset);
        initDTS = Math.min(initDTS,videoSamples[0].dts - pesTimeScale * timeOffset);
+        this.observer.trigger(Event.INIT_PTS_FOUND, {initPTS});


could you change the dispatched object (for consistency) as being something like :
{ id : demuxerId, initPTS: initPTS}

Yes good point will patch tomorrow

mangui · 2016-12-02T09:30:07Z

src/controller/audio-stream-controller.js

+    //Always update the new INIT PTS
+    //Can change due level switch
+    this.initPTS = data.initPTS;
+    logger(`InitPTS , ${this.initPTS}, found from video track`);


s/logger/logger.log

… INIT_PTS_FOUND

mangui · 2016-12-02T16:03:47Z

this PR breaks support of alt audio VoD stream with discontinuity such as
https://hls.ted.com/talks/2024.m3u8?qr&network=tv&preroll=Blank

mangui · 2016-12-02T16:23:01Z

initPTS should be linked to a continuity counter index.
looking into it

mangui · 2016-12-06T16:46:11Z

closing in favor of https://github.com/dailymotion/hls.js/pull/860 that also contains these changes

fredvb and others added 7 commits October 6, 2016 12:29

simpleton live alt-audio support inspired by the stream-controller.

76c45a2

Merge remote-tracking branch 'upstream/master'

b9173de

Merge pull request video-dev#1 from dailymotion/master

05db3e6

merge

Initial working live alt audio

58309d3

Patching lint complains

ae385b3

live alternative audio

414326e

inspired on fredvb pull request 727 and INIT_PTS_FOUND on pull request 700 using 0.6.10 base code

lint complains

fdb74ea

Sergio Monteiro added 3 commits November 29, 2016 17:13

Merging with lattest code from master on v0.6.12

33b9910

Merge remote-tracking branch 'upstream/master' into live-alt-audio-in…

d8d47f3

…spired_on_fredvb_and_pull_700

mangui reviewed Dec 1, 2016

View reviewed changes

mangui reviewed Dec 2, 2016

View reviewed changes

mangui and others added 3 commits December 2, 2016 10:31

audio-stream-controller: fix syntax error

101ad9c

audio-stream-controller: fix another syntax error

8879b74

Changing to include demuxer id dispatched object (for consistency) on…

2466d85

… INIT_PTS_FOUND

mangui closed this Dec 6, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Live alt audio inspired on fredvb and pull 700 #832

Live alt audio inspired on fredvb and pull 700 #832

sergiojm commented Nov 20, 2016

mangui commented Nov 21, 2016

mangui left a comment

mangui Dec 1, 2016

sergiojgm Dec 1, 2016

mangui Dec 1, 2016

sergiojgm Dec 1, 2016 •

edited

Loading

mangui Dec 1, 2016

sergiojgm Dec 1, 2016

mangui Dec 1, 2016

sergiojgm Dec 1, 2016

mangui Dec 1, 2016

sergiojgm Dec 1, 2016

mangui Dec 2, 2016

mangui commented Dec 2, 2016

mangui commented Dec 2, 2016

mangui commented Dec 6, 2016


		var currentTime=this.media.currentTime;

		// When switching audio track the bufferEnd should only be considered

Live alt audio inspired on fredvb and pull 700 #832

Live alt audio inspired on fredvb and pull 700 #832

Conversation

sergiojm commented Nov 20, 2016

mangui commented Nov 21, 2016

mangui left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergiojgm Dec 1, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mangui commented Dec 2, 2016

mangui commented Dec 2, 2016

mangui commented Dec 6, 2016

sergiojgm Dec 1, 2016 •

edited

Loading