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

updateSettings new streaming parameters : trackSwitchMode and selectionModeForInitialTrack #3385

Conversation

jeffcunat
Copy link
Contributor

This PR adds two new settings parameters (may have been forgotten when new updateSettings function was created in v3)

{ streaming: 
  {
    trackSwitchMode: {audio: audioMode , video: videoMode},
    selectionModeForInitialTrack: mode,
  }
}

and deprecates setTrackSwitchModeFor and setSelectionModeForInitialTrack functions (still usable though)

@dsilhavy dsilhavy added this to the 3.1.4 milestone Sep 8, 2020
@dsilhavy
Copy link
Collaborator

dsilhavy commented Oct 7, 2020

@jeffcunat Thanks for this PR. Minor issues:

  • BufferController and ScheduleController still have a reference to MediaController.TRACK_SWITCH_MODE_ALWAYS_REPLACE instead of Constants.TRACK_SWITCH_MODE_ALWAYS_REPLACE
  • Same for unit/streaming.controllers.MediaController.js

@jeffcunat jeffcunat force-pushed the update-settings-missing-parameters branch from 652ed9c to e9f8fa0 Compare October 13, 2020 06:51
@jeffcunat
Copy link
Contributor Author

@dsilhavy I have fixed the minor issues you mentioned.
What is also different with this PR is that only audio and video type can be used for the new settings streaming.trackSwitchMode (before any type could be used). It seems that it is not relevant for text. But could you confirm ?

Unit tests were done with a fake 'test' type so now I have also added specific audio and video unit tests that test default settings values and their update.

@dsilhavy
Copy link
Collaborator

@jeffcunat Thanks

Regarding your question: BufferController also checks the trackSwitchMode for fragmentedText. However, ranges.length is 0 for the streams I tested so it does not really affect anything. I think it is fine to limit this to audio and video.

@dsilhavy dsilhavy merged commit 70c4b84 into Dash-Industry-Forum:development Oct 13, 2020
@bbert bbert deleted the update-settings-missing-parameters branch October 28, 2020 15:44
bbert pushed a commit to Orange-OpenSource/dash.js that referenced this pull request Nov 4, 2020
…onModeForInitialTrack (Dash-Industry-Forum#3385)

* introduce new streaming.trackSwitchMode settings

* new streaming.selectionModeForInitialTrack settings

* missing replacement of Constants.TRACK_SWITCH_MODE_ALWAYS_REPLACE

* fix unit tests

* add unit test for setSwitchMode with unsupported type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants