diff --git a/index.html b/index.html index bb573f08..ec82b657 100755 --- a/index.html +++ b/index.html @@ -29,6 +29,7 @@ Feedback | GitHub

+ diff --git a/js/actionCreators.js b/js/actionCreators.js index 64a5576a..d31b5466 100644 --- a/js/actionCreators.js +++ b/js/actionCreators.js @@ -1,9 +1,11 @@ -import jsmediatags from "jsmediatags/dist/jsmediatags"; import { parser, creator } from "winamp-eqf"; import { genArrayBufferFromFileReference, genArrayBufferFromUrl, - promptForFileReferences + promptForFileReferences, + genMediaDuration, + genMediaTags, + genAudioFileUrlsFromDropbox } from "./fileUtils"; import skinParser from "./skinParser"; import { @@ -27,7 +29,8 @@ import { base64FromArrayBuffer, downloadURI, normalize, - sort + sort, + debounce } from "./utils"; import { CLOSE_WINAMP, @@ -61,9 +64,17 @@ import { TOGGLE_SHADE_MODE, TOGGLE_PLAYLIST_SHADE_MODE, MEDIA_TAG_REQUEST_INITIALIZED, - MEDIA_TAG_REQUEST_FAILED + MEDIA_TAG_REQUEST_FAILED, + PLAYLIST_SIZE_CHANGED } from "./actionTypes"; +import LoadQueue from "./loadQueue"; + +const DURATION_PRIORITY = 5; +const META_DATA_PRIORITY = 10; + +const loadQueue = new LoadQueue({ threads: 4 }); + function playRandomTrack() { return (dispatch, getState) => { const { playlist: { trackOrder, currentTrack } } = getState(); @@ -237,22 +248,14 @@ export function loadFilesFromReferences( export function fetchMediaDuration(url, id) { return dispatch => { - // TODO: Does this actually stop downloading the file once it's - // got the duration? - const audio = document.createElement("audio"); - audio.crossOrigin = "anonymous"; - const durationChange = () => { - const { duration } = audio; - dispatch({ type: SET_MEDIA_DURATION, duration, id }); - audio.removeEventListener("durationchange", durationChange); - audio.url = null; - // TODO: Not sure if this really gets cleaned up. - }; - audio.addEventListener("durationchange", durationChange); - audio.addEventListener("error", () => { - // TODO: Should we update the state to indicate that we don't know the length? - }); - audio.src = url; + loadQueue.push(() => { + return genMediaDuration(url) + .then(duration => dispatch({ type: SET_MEDIA_DURATION, duration, id })) + .catch(() => { + // TODO: Should we update the state to indicate that we don't know the length? + }); + // TODO: The priority should depend upon visiblity + }, () => DURATION_PRIORITY); }; } @@ -313,52 +316,51 @@ export function loadMediaFile(track, priority = null, atIndex = 0) { if (metaData != null) { const { artist, title } = metaData; dispatch({ type: SET_MEDIA_TAGS, artist, title, id }); + } else if (blob != null) { + // Blobs can be loaded quickly + dispatch(fetchMediaTags(blob, id)); } else { - dispatch(fetchMediaTags(url || blob, id)); + dispatch(fetchMediaTagsForVisibleTracks()); } }; } +const _fetchMediaTagsForVisibleTracksThunk = debounce((dispatch, getState) => { + getVisibleTracks(getState()) + .filter( + track => + track.mediaTagsRequestStatus === MEDIA_TAG_REQUEST_STATUS.NOT_REQUESTED + ) + .forEach(track => { + loadQueue.push( + () => dispatch(fetchMediaTags(track.url, track.id)), + () => META_DATA_PRIORITY + ); + }); +}, 200); + export function fetchMediaTagsForVisibleTracks() { - return (dispatch, getState) => { - getVisibleTracks(getState()) - .filter( - track => - track.mediaTagsRequestStatus === - MEDIA_TAG_REQUEST_STATUS.NOT_REQUESTED - ) - .forEach(track => { - dispatch(fetchMediaTags(track.url, track.id)); - }); - }; + return _fetchMediaTagsForVisibleTracksThunk; } +export const debouncedFetchMediaTagsForVisibleTracks = debounce( + fetchMediaTagsForVisibleTracks, + 200 +); + export function fetchMediaTags(file, id) { - // Workaround https://github.com/aadsm/jsmediatags/issues/83 - if (typeof file === "string" && !/^[a-z]+:\/\//i.test(file)) { - file = `${location.protocol}//${location.host}${location.pathname}${file}`; - } return dispatch => { dispatch({ type: MEDIA_TAG_REQUEST_INITIALIZED, id }); - try { - jsmediatags.read(file, { - onSuccess: data => { - const { artist, title } = data.tags; - // There's more data here, but we don't have a use for it yet: - // https://github.com/aadsm/jsmediatags#shortcuts - dispatch({ type: SET_MEDIA_TAGS, artist, title, id }); - }, - onError: () => { - dispatch({ type: MEDIA_TAG_REQUEST_FAILED, id }); - // Nothing to do. The filename will have to suffice. - } + return genMediaTags(file) + .then(data => { + const { artist, title } = data.tags; + // There's more data here, but we don't have a use for it yet: + // https://github.com/aadsm/jsmediatags#shortcuts + dispatch({ type: SET_MEDIA_TAGS, artist, title, id }); + }) + .catch(() => { + dispatch({ type: MEDIA_TAG_REQUEST_FAILED, id }); }); - } catch (e) { - // Possibly jsmediatags could not find a parser for this file? - // Nothing to do. - // Consider removing this after https://github.com/aadsm/jsmediatags/issues/83 is resolved. - dispatch({ type: MEDIA_TAG_REQUEST_FAILED, id }); - } }; } @@ -415,23 +417,6 @@ export function openSkinFileDialog() { return _openFileDialog(".zip, .wsz"); } -// Requires Dropbox's Chooser to be loaded on the page -function genAudioFileUrlsFromDropbox() { - return new Promise((resolve, reject) => { - if (window.Dropbox == null) { - reject(); - } - window.Dropbox.choose({ - success: resolve, - error: reject, - linkType: "direct", - folderselect: false, - multiselect: true, - extensions: ["video", "audio"] - }); - }); -} - export function openDropboxFileDialog() { return async dispatch => { const files = await genAudioFileUrlsFromDropbox(); @@ -566,7 +551,17 @@ export function toggleVisualizerStyle() { } export function setPlaylistScrollPosition(position) { - return { type: SET_PLAYLIST_SCROLL_POSITION, position }; + return dispatch => { + dispatch({ type: SET_PLAYLIST_SCROLL_POSITION, position }); + dispatch(fetchMediaTagsForVisibleTracks()); + }; +} + +export function setPlaylistSize(size) { + return dispatch => { + dispatch({ type: PLAYLIST_SIZE_CHANGED, size }); + dispatch(fetchMediaTagsForVisibleTracks()); + }; } export function scrollNTracks(n) { diff --git a/js/components/MainWindow/MainContextMenu.js b/js/components/MainWindow/MainContextMenu.js index f9ff8757..485698f4 100644 --- a/js/components/MainWindow/MainContextMenu.js +++ b/js/components/MainWindow/MainContextMenu.js @@ -5,7 +5,8 @@ import { close, setSkinFromUrl, openMediaFileDialog, - openSkinFileDialog + openSkinFileDialog, + openDropboxFileDialog } from "../../actionCreators"; import { ContextMenu, Hr, Node, Parent, LinkNode } from "../ContextMenu"; @@ -21,7 +22,10 @@ const MainContextMenu = props => ( label="Winamp2-js" />
- + + + + {!!props.avaliableSkins.length &&
} @@ -46,6 +50,7 @@ const mapDispatchToProps = { close, openSkinFileDialog, openMediaFileDialog, + openDropboxFileDialog, setSkin: setSkinFromUrl }; diff --git a/js/components/PlaylistWindow/PlaylistResizeTarget.js b/js/components/PlaylistWindow/PlaylistResizeTarget.js index 09837ebc..3ab02097 100644 --- a/js/components/PlaylistWindow/PlaylistResizeTarget.js +++ b/js/components/PlaylistWindow/PlaylistResizeTarget.js @@ -1,6 +1,6 @@ import { connect } from "react-redux"; import ResizeTarget from "../ResizeTarget"; -import { PLAYLIST_SIZE_CHANGED } from "../../actionTypes"; +import { setPlaylistSize } from "../../actionCreators"; import { PLAYLIST_RESIZE_SEGMENT_WIDTH, PLAYLIST_RESIZE_SEGMENT_HEIGHT @@ -13,8 +13,6 @@ const mapStateToProps = state => ({ id: "playlist-resize-target" }); -const mapDispatchToProps = { - setPlaylistSize: size => ({ type: PLAYLIST_SIZE_CHANGED, size }) -}; +const mapDispatchToProps = { setPlaylistSize }; export default connect(mapStateToProps, mapDispatchToProps)(ResizeTarget); diff --git a/js/components/PlaylistWindow/index.js b/js/components/PlaylistWindow/index.js index ae725a30..6c961827 100644 --- a/js/components/PlaylistWindow/index.js +++ b/js/components/PlaylistWindow/index.js @@ -48,6 +48,7 @@ class PlaylistWindow extends React.Component { _handleDrop(e, targetCoords) { const top = e.clientY - targetCoords.y; + // TODO: Include the scroll offset in this const atIndex = clamp( Math.round((top - 23) / TRACK_HEIGHT), 0, diff --git a/js/fileUtils.js b/js/fileUtils.js index 77f1986c..d295ed09 100644 --- a/js/fileUtils.js +++ b/js/fileUtils.js @@ -1,4 +1,73 @@ +import invariant from "invariant"; +import jsmediatags from "jsmediatags/dist/jsmediatags"; + +// Requires Dropbox's Chooser to be loaded on the page +export function genAudioFileUrlsFromDropbox() { + return new Promise((resolve, reject) => { + if (window.Dropbox == null) { + reject(); + } + window.Dropbox.choose({ + success: resolve, + error: reject, + linkType: "direct", + folderselect: false, + multiselect: true, + extensions: ["video", "audio"] + }); + }); +} + +export function genMediaTags(file) { + invariant( + file != null, + "Attempted to get the tags of media file without passing a file" + ); + // Workaround https://github.com/aadsm/jsmediatags/issues/83 + if (typeof file === "string" && !/^[a-z]+:\/\//i.test(file)) { + file = `${location.protocol}//${location.host}${location.pathname}${file}`; + } + return new Promise((resolve, reject) => { + try { + jsmediatags.read(file, { onSuccess: resolve, onError: reject }); + } catch (e) { + // Possibly jsmediatags could not find a parser for this file? + // Nothing to do. + // Consider removing this after https://github.com/aadsm/jsmediatags/issues/83 is resolved. + reject(e); + } + }); +} + +export function genMediaDuration(url) { + invariant( + typeof url === "string", + "Attempted to get the duration of media file without passing a url" + ); + return new Promise((resolve, reject) => { + // TODO: Does this actually stop downloading the file once it's + // got the duration? + const audio = document.createElement("audio"); + audio.crossOrigin = "anonymous"; + const durationChange = () => { + resolve(audio.duration); + audio.removeEventListener("durationchange", durationChange); + audio.url = null; + // TODO: Not sure if this really gets cleaned up. + }; + audio.addEventListener("durationchange", durationChange); + audio.addEventListener("error", e => { + reject(e); + }); + audio.src = url; + }); +} + export async function genArrayBufferFromFileReference(fileReference) { + invariant( + fileReference != null, + "Attempted to get an ArrayBuffer without assing a fileReference" + ); return new Promise((resolve, reject) => { const reader = new FileReader(); reader.onload = function(e) { diff --git a/js/loadQueue.js b/js/loadQueue.js new file mode 100644 index 00000000..99e323bb --- /dev/null +++ b/js/loadQueue.js @@ -0,0 +1,47 @@ +import invariant from "invariant"; +import TinyQueue from "tinyqueue"; + +// Push promises onto a queue with a priority. +// Run a given number of jobs in parallel +// Useful for prioritizing network requests +export default class LoadQueue { + constructor({ threads }) { + // TODO: Consider not running items with zero prioirty + // Priority is a function so that items can change their priority between + // when their priority is evaluated. + // For example, we might add a track to the playlist and then scroll to/away + // from it before it gets processed. + this._queue = new TinyQueue([], (a, b) => a.priority() > b.priority()); + this._avaliableThreads = threads; + } + + push(task, priority) { + const t = { task, priority }; + this._queue.push(t); + this._run(); + return () => { + // TODO: Could return a boolean representing if the task has already been + // kicke off. + this._queue = this._queue.filter(t1 => t1 !== t); + }; + } + + _run() { + while (this._avaliableThreads > 0) { + if (this._queue.length === 0) { + return; + } + this._avaliableThreads--; + const t = this._queue.pop(); + const promise = t.task(); + invariant( + typeof promise.then === "function", + `LoadQueue only supports loading Promises. Got ${promise}` + ); + promise.then(() => { + this._avaliableThreads++; + this._run(); + }); + } + } +} diff --git a/js/reducers/playlist.js b/js/reducers/playlist.js index 4f5313a9..2ec16f89 100644 --- a/js/reducers/playlist.js +++ b/js/reducers/playlist.js @@ -148,6 +148,7 @@ const playlist = (state = defaultPlaylistState, action) => { tracks: { ...state.tracks, [action.id]: { + id: action.id, selected: false, defaultName: action.defaultName, duration: null, diff --git a/js/reducers/playlist.test.js b/js/reducers/playlist.test.js index bba91498..6ea67d8e 100644 --- a/js/reducers/playlist.test.js +++ b/js/reducers/playlist.test.js @@ -22,6 +22,7 @@ describe("playlist reducer", () => { expect(nextState).toEqual({ tracks: { 100: { + id: 100, selected: false, duration: null, defaultName: "My Track Name", @@ -36,8 +37,8 @@ describe("playlist reducer", () => { it("defaults to adding new tracks to the end of the list", () => { const initialState = { tracks: { - 2: { selected: false }, - 3: { selected: false } + 2: { id: 2, selected: false }, + 3: { id: 3, selected: false } }, trackOrder: [3, 2], lastSelectedIndex: 0 @@ -50,9 +51,10 @@ describe("playlist reducer", () => { }); expect(nextState).toEqual({ tracks: { - 2: { selected: false }, - 3: { selected: false }, + 2: { id: 2, selected: false }, + 3: { id: 3, selected: false }, 100: { + id: 100, selected: false, duration: null, mediaTagsRequestStatus: "NOT_REQUESTED", @@ -67,8 +69,8 @@ describe("playlist reducer", () => { it("can handle adding a track at a given index", () => { const initialState = { tracks: { - 2: { selected: false }, - 3: { selected: false } + 2: { id: 2, selected: false }, + 3: { id: 3, selected: false } }, trackOrder: [3, 2], lastSelectedIndex: 0 @@ -82,9 +84,10 @@ describe("playlist reducer", () => { }); expect(nextState).toEqual({ tracks: { - 2: { selected: false }, - 3: { selected: false }, + 2: { id: 2, selected: false }, + 3: { id: 3, selected: false }, 100: { + id: 100, selected: false, duration: null, mediaTagsRequestStatus: "NOT_REQUESTED", diff --git a/js/utils.js b/js/utils.js index 377bf5d7..db8b22fd 100644 --- a/js/utils.js +++ b/js/utils.js @@ -232,3 +232,15 @@ export const arrayWithout = (arr, value) => { s["delete"](value); return Array.from(s); }; + +export function debounce(func, delay) { + let token; + return function(...args) { + if (token != null) { + clearTimeout(token); + } + token = setTimeout(() => { + func.apply(this, args); + }, delay); + }; +} diff --git a/loadQueue.test.js b/loadQueue.test.js new file mode 100644 index 00000000..8bf5c823 --- /dev/null +++ b/loadQueue.test.js @@ -0,0 +1,20 @@ +import LoadQueue from "./loadQueue"; + +describe("LoadQueue", () => { + it("executes some promises", () => { + const results = []; + const makePushPromise = n => { + return new Promise(resolve => { + results.push(n); + resolve(); + }); + }; + + const task1 = makePushPromise(1); + const task2 = makePushPromise(2); + const task3 = makePushPromise(3); + const task4 = makePushPromise(4); + + const loadQueue = new LoadQueue({ threads: 4 }); + }); +}); diff --git a/package.json b/package.json index 918cfacf..56e2e78f 100644 --- a/package.json +++ b/package.json @@ -68,6 +68,7 @@ "cardinal-spline-js": "^2.3.6", "classnames": "^2.2.5", "eslint-plugin-import": "^2.7.0", + "invariant": "^2.2.3", "jest": "^22.0.3", "jsmediatags": "^3.8.1", "jszip": "^3.1.3", @@ -82,6 +83,7 @@ "redux-devtools-extension": "^2.13.2", "redux-thunk": "^2.1.0", "reselect": "^3.0.1", + "tinyqueue": "^1.2.3", "webpack": "^3.6.0", "winamp-eqf": "^1.0.0" }, diff --git a/yarn.lock b/yarn.lock index c4db5113..81b43b6c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3152,6 +3152,12 @@ invariant@^2.0.0, invariant@^2.2.2: dependencies: loose-envify "^1.0.0" +invariant@^2.2.3: + version "2.2.3" + resolved "https://registry.yarnpkg.com/invariant/-/invariant-2.2.3.tgz#1a827dfde7dcbd7c323f0ca826be8fa7c5e9d688" + dependencies: + loose-envify "^1.0.0" + invert-kv@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/invert-kv/-/invert-kv-1.0.0.tgz#104a8e4aaca6d3d8cd157a8ef8bfab2d7a3ffdb6" @@ -6208,6 +6214,10 @@ timers-browserify@^2.0.4: dependencies: setimmediate "^1.0.4" +tinyqueue@^1.2.3: + version "1.2.3" + resolved "https://registry.yarnpkg.com/tinyqueue/-/tinyqueue-1.2.3.tgz#b6a61de23060584da29f82362e45df1ec7353f3d" + tmp@^0.0.33: version "0.0.33" resolved "https://registry.yarnpkg.com/tmp/-/tmp-0.0.33.tgz#6d34335889768d21b2bcda0aa277ced3b1bfadf9"