feat: Support switching codecs if supported by the browser #841

brandonocasey · 2020-05-06T19:57:39Z

If the browser supports sourceBuffer.changeType stop excluding playlists that have a different codec than the initial selected rendition. If we switch to a playlist that has a different codec than the current rendition change codecs using changeType

index.html

scripts/index-demo-page.js

brandonocasey · 2020-05-15T18:04:40Z

src/master-playlist-controller.js

@@ -589,15 +599,15 @@ export class MasterPlaylistController extends videojs.EventTarget {
   *
   * @private
   */
-  smoothQualityChange_() {
-    const media = this.selectPlaylist();
+  smoothQualityChange_(media = this.selectPlaylist()) {


Allows us to easily change the media that is currently being used

brandonocasey · 2020-05-15T18:05:00Z

src/master-playlist-controller.js

-      }
-      return;
-    }
+    this.tryToCreateSourceBuffers_();


We won't throw an error that can be caught by this anymore

brandonocasey · 2020-05-15T18:09:11Z

src/media-segment-request.js

@@ -290,7 +290,8 @@ const transmuxAndNotify = ({
    if (probeResult) {
      trackInfoFn(segment, {
        hasAudio: probeResult.hasAudio,
-        hasVideo: probeResult.hasVideo
+        hasVideo: probeResult.hasVideo,
+        isMuxed


When messing around with renditions in safari I realized that we don't pass isMuxed up for ts segments. This is a problem because ts segments can contain video/audio and be appended to a "videoBuffer" that has both codecs.

brandonocasey · 2020-05-15T18:10:48Z

src/segment-loader.js

@@ -450,6 +490,12 @@ export default class SegmentLoader extends videojs.EventTarget {
      }
    });

+    this.on('trackinfo', () => {


Since startingMedia_ is reset on media change now, we have to check if we have enough to append after trackinfo, which sets startingMedia_

src/rendition-mixin.js

brandonocasey · 2020-05-15T18:11:26Z

src/segment-loader.js

    // When we have track info, determine what media types this loader is dealing with.
    // Guard against cases where we're not getting track info at all until we are
    // certain that all streams will provide it.
-    if (typeof this.startingMedia_ === 'undefined' && (trackInfo.hasAudio || trackInfo.hasVideo)) {
+    if ((trackInfo.hasVideo || trackInfo.hasAudio) && !shallowEqual(this.startingMedia_, trackInfo)) {


If the trackinfo actually changed fire a new trackinfo event.

src/videojs-http-streaming.js

brandonocasey · 2020-05-15T18:12:46Z

test/manifests/dash-many-codecs.mpd

@@ -3,7 +3,7 @@
 <Period>
  <AdaptationSet mimeType="video/mp4" contentType="video" subsegmentAlignment="true" subsegmentStartsWithSAP="1" par="16:9">
   <SegmentTemplate duration="120" timescale="30" media="$RepresentationID$/$RepresentationID$_$Number$.m4v" startNumber="1" initialization="$RepresentationID$/$RepresentationID$_0.m4v"/>
-   <Representation id="video_1" codecs="avc1.foo.bar" bandwidth="507246" width="320" height="180" frameRate="30" sar="1:1" scanType="progressive"/>
+   <Representation id="video_1" codecs="avc1.4d400d" bandwidth="507246" width="320" height="180" frameRate="30" sar="1:1" scanType="progressive"/>


These have to be real codecs now, as we don't blacklist on codec change.

gkatsev

Don't understand the code that much still but LGTM.
One thing to note and not sure if it's a Safari thing or what, but I was test with the "Advanced Bipbop - Fmp4 hevc, demuxed" source and it cannot play continuously on safari.

gkatsev · 2020-05-29T16:42:03Z

src/media-groups.js


-  mediaTypes.AUDIO.tracks[groupId].enabled = true;
-  mediaTypes.AUDIO.onTrackChanged();
+  if (audioGroup) {


gesinger

Will review the tests soon, but wanted to get the comments in for the non tests portion.

src/master-playlist-controller.js

gesinger · 2020-05-21T19:02:10Z

src/master-playlist-controller.js

-    // Therefore we must always wait for the segment loader's track info.
-    if (!mainStartingMedia || (hasAltAudio && !this.audioSegmentLoader_.startingMedia_)) {
+    // no codecs, no playback.
+    if (!codecs.audio && !codecs.video) {


Could this blacklist playlists if content hasn't been downloaded when we try to create the source buffers?

I will see if I can add a check to make sure we have startingMedia at the top of this function, so that won't be possible.

src/master-playlist-controller.js

src/source-updater.js

gesinger · 2020-06-15T13:19:40Z

src/source-updater.js

+   *        The codec string to change type with on the source buffer.
+   */
+  changeType(type, codec) {
+    if (!this.canChangeType()) {


Can this condition be reached by our code?

No we always check for this.canChangeType before trying to call changeType. Do you think we should remove this line or throw an Error here?

src/source-updater.js

src/master-playlist-controller.js

gesinger · 2020-06-15T14:17:21Z

src/segment-loader.js

-      return;
+      if (changed) {
+        this.logger_('trackinfo update', trackInfo);
+        this.trigger('trackinfo');


Would it be worth renaming to newtrackinfo for clarity?

perhaps: trackinfo and trackinfochange. I don't know if we should other than to prevent breaking people that may have been using the event.

src/master-playlist-controller.js

brandonocasey · 2020-06-22T21:02:29Z

src/master-playlist-controller.js

@@ -553,7 +553,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
      const codecs = this.getCodecsOrExclude_();

      // no codecs means that the playlist was excluded
-      if (!codecs || !this.sourceUpdater_.canChangeType()) {
+      if (!codecs) {


We do not want to check canChangeType here, as we do it in getCodecsOrExclude and blacklist if the main part of the codec was changed, but sourcebuffers can not actually changetype.

brandonocasey · 2020-06-22T21:04:40Z

src/master-playlist-controller.js

@@ -1350,6 +1346,9 @@ export class MasterPlaylistController extends videojs.EventTarget {
    if (usingAudioLoader && unsupportedAudio && this.media().attributes.AUDIO) {
      const audioGroup = this.media().attributes.AUDIO;

+      this.mediaTypes_.AUDIO.activePlaylistLoader.pause();


This is important because if we wait for the masterPlaylistLoader to mediachange and then call abort then we will already have appended unsupported codecs.

brandonocasey commented May 6, 2020

View reviewed changes

index.html Outdated Show resolved Hide resolved

brandonocasey commented May 6, 2020

View reviewed changes

scripts/index-demo-page.js Outdated Show resolved Hide resolved

brandonocasey force-pushed the feat/codec-switching branch from 555f42d to 2a88916 Compare May 8, 2020 15:39

brandonocasey commented May 15, 2020

View reviewed changes

scripts/index-demo-page.js Outdated Show resolved Hide resolved

brandonocasey commented May 15, 2020

View reviewed changes

src/rendition-mixin.js Outdated Show resolved Hide resolved

brandonocasey commented May 15, 2020

View reviewed changes

src/videojs-http-streaming.js Outdated Show resolved Hide resolved

brandonocasey commented May 15, 2020

View reviewed changes

brandonocasey force-pushed the feat/codec-switching branch from da1e642 to d14d7f2 Compare May 18, 2020 19:00

gkatsev added this to the 2.1 milestone May 21, 2020

gkatsev reviewed Jun 12, 2020

View reviewed changes

src/media-groups.js

mediaTypes.AUDIO.tracks[groupId].enabled = true;

mediaTypes.AUDIO.onTrackChanged();

if (audioGroup) {

Copy link

Member

gkatsev May 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

gesinger reviewed Jun 15, 2020

View reviewed changes

gkatsev changed the base branch from master to main June 19, 2020 16:50

brandonocasey force-pushed the feat/codec-switching branch 2 times, most recently from c148bca to 2657730 Compare June 22, 2020 18:13

brandonocasey commented Jun 22, 2020

View reviewed changes

gkatsev previously approved these changes Jun 24, 2020

View reviewed changes

brandonocasey added 8 commits June 25, 2020 17:08

feat: Support switching codecs if supported by the browser

df30c63

ie 11 support...

1ea8a52

bug squash

1ac680a

test fixes

6c3ad84

null out startingMedia_ in segmentLoader

d8ca1c5

ie 11 fix

2e51e75

add playlist to representations

79c68f9

more fixes

c339569

brandonocasey added 19 commits June 25, 2020 17:11

tests

6a9a594

finish unit test

81b93b9

fix type mocking

da976d7

fix for ie 11

16e51ab

more ie 11 fixes

58abdbb

smaller code changes

67629ca

:sigh: more ie 11 fixes

8e152b9

move things to other pull requests

2960558

remove canCodecSwitch, rename codecSwitch to addOrChangeSourceBuffers

181ce6f

fix test

37109fe

comment updates from cr

97395e4

move to helpers, buffered -> bufferedIntersection

b22beb4

export default on shallow equal

796daf5

small refactor

9e40330

bring back firefox fix

2ff720e

botched rebase

6132f32

test fixes due to starting media check in getCodecsOrExclude

07b7403

add comment

25b70fb

remove currentMedia, abort/pause audio loaders on exclude

1b9637b

brandonocasey dismissed gkatsev’s stale review via 1b9637b June 25, 2020 21:14

brandonocasey force-pushed the feat/codec-switching branch from ae6e9c2 to 1b9637b Compare June 25, 2020 21:14

rebase test failure

067b86e

brandonocasey merged commit 267cc34 into main Jun 25, 2020

gkatsev deleted the feat/codec-switching branch June 26, 2020 14:33

gsimko mentioned this pull request Feb 4, 2023

Fragmented MP4 always leads to Video/Audio append errors when switching quality. #1344

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support switching codecs if supported by the browser #841

feat: Support switching codecs if supported by the browser #841

brandonocasey commented May 6, 2020

brandonocasey May 15, 2020

brandonocasey May 15, 2020

brandonocasey May 15, 2020

brandonocasey May 15, 2020

brandonocasey May 15, 2020

brandonocasey May 15, 2020

gkatsev left a comment

gkatsev May 29, 2020

gesinger left a comment

gesinger May 21, 2020

brandonocasey Jun 18, 2020

brandonocasey Jun 22, 2020

gesinger Jun 15, 2020

brandonocasey Jun 17, 2020

gesinger Jun 15, 2020

brandonocasey Jun 22, 2020

brandonocasey Jun 22, 2020

brandonocasey Jun 22, 2020

feat: Support switching codecs if supported by the browser #841

feat: Support switching codecs if supported by the browser #841

Conversation

brandonocasey commented May 6, 2020

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

gkatsev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gesinger 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

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