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

Setting loudness to default volume in new videos #3203

Merged
merged 7 commits into from
Mar 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions src/renderer/components/ft-video-player/ft-video-player.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export default defineComponent({
id: '',
powerSaveBlocker: null,
volume: 1,
muted: false,
player: null,
useDash: false,
useHls: false,
Expand Down Expand Up @@ -305,11 +306,18 @@ export default defineComponent({
},
mounted: function () {
const volume = sessionStorage.getItem('volume')
const muted = sessionStorage.getItem('muted')

if (volume !== null) {
this.volume = volume
}

if (muted !== null) {
// as sessionStorage stores string values which are truthy by default so we must check with 'true'
// otherwise 'false' will be returned as true as well
this.muted = (muted === 'true')
}

this.dataSetup.playbackRates = this.playbackRates

this.createFullWindowButton()
Expand Down Expand Up @@ -390,6 +398,7 @@ export default defineComponent({
})

this.player.volume(this.volume)
this.player.muted(this.muted)
this.player.playbackRate(this.defaultPlayback)
this.player.textTrackSettings.setValues(this.defaultCaptionSettings)
// Remove big play button
Expand Down Expand Up @@ -699,10 +708,21 @@ export default defineComponent({
},

updateVolume: function (_event) {
// 0 means muted
// https://docs.videojs.com/html5#volume
const volume = this.player.muted() ? 0 : this.player.volume()
sessionStorage.setItem('volume', volume)
if (sessionStorage.getItem('muted') === 'false' && this.player.volume() === 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Why are you checking the session storage here, instead of the player?

Copy link
Contributor Author

@predystopic-dev predystopic-dev Feb 20, 2023

Choose a reason for hiding this comment

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

sessionStorage is not set to true when we drag the slider to 0 (it does get set to true if we press 'm' or click on mute though) so in those cases I am setting volume to defaultVolume set by user in settings. This should happen when updateVolume function is called by volumechange event I reckon? Because if these conditions are checked in player, then the feature doesn't work as intended

Copy link
Member

Choose a reason for hiding this comment

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

Video.js doesn't change anything in session storage. the values in session storage are set once at startup by the code in the settings file and then updated by the code in this function every time the player is muted/unmuted or the volume of the player changes. Without this updateVolume function nothing would be changed in session storage after setting the initial values at startup in the settings file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are there any changes that I should do here? I tried checking sessionStorage in player and update player volume accordingly but it doesn't work as intended.

// If video is muted by dragging volume slider, it doesn't change 'muted' in sessionStorage to true
// hence compare it with 'false' and set volume to defaultVolume.
const volume = parseFloat(sessionStorage.getItem('defaultVolume'))
absidue marked this conversation as resolved.
Show resolved Hide resolved
const muted = true
sessionStorage.setItem('volume', volume)
sessionStorage.setItem('muted', muted)
} else {
// If volume isn't muted by dragging the slider, muted and volume values are carried over to next video.
const volume = this.player.volume()
absidue marked this conversation as resolved.
Show resolved Hide resolved
const muted = this.player.muted()
sessionStorage.setItem('volume', volume)
sessionStorage.setItem('muted', muted)
}
},

mouseScrollVolume: function (event) {
Expand Down
2 changes: 2 additions & 0 deletions src/renderer/store/modules/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,8 @@ const stateWithSideEffects = {
defaultValue: 1,
sideEffectsHandler: (_, value) => {
sessionStorage.setItem('volume', value)
value === 0 ? sessionStorage.setItem('muted', 'true') : sessionStorage.setItem('muted', 'false')
sessionStorage.setItem('defaultVolume', value)
}
},

Expand Down