-
Notifications
You must be signed in to change notification settings - Fork 681
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
Integration with music-metadata #606
Changes from 19 commits
475f1f6
e7fd0b4
646bd11
33673a6
c8be1ff
800e156
c988ef8
37c4273
e364610
472c2c0
963e0ee
aff1c18
27241ba
f21cf3a
d086326
acac8bc
ae13e55
2c6270f
a6ac1fd
d56acd7
d927b3d
9e137c6
25ba886
edfdcaf
7d72395
4cee6ae
8159a13
1005a4d
7fd8740
a4e240c
ba389e3
d210971
032a770
cff4858
b06c8b7
5f1fbe7
1070c15
3a2e5ae
b52b46e
fc5bcb6
6bfde63
ba217b8
f3d165a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,8 @@ import { | |
SET_MEDIA_DURATION, | ||
MEDIA_TAG_REQUEST_INITIALIZED, | ||
MEDIA_TAG_REQUEST_FAILED, | ||
SET_SKIN_DATA | ||
SET_SKIN_DATA, | ||
SET_MEDIA | ||
} from "../actionTypes"; | ||
import LoadQueue from "../loadQueue"; | ||
|
||
|
@@ -169,7 +170,7 @@ export function loadMediaFiles(tracks, loadStyle = null, atIndex = 0) { | |
export function loadMediaFile(track, priority = null, atIndex = 0) { | ||
return dispatch => { | ||
const id = uniqueId(); | ||
const { url, blob, defaultName, metaData, duration } = track; | ||
const { url, blob, defaultName, metaData } = track; | ||
let canonicalUrl = url; | ||
if (canonicalUrl == null) { | ||
if (blob == null) { | ||
|
@@ -192,14 +193,6 @@ export function loadMediaFile(track, priority = null, atIndex = 0) { | |
case LOAD_STYLE.PLAY: | ||
dispatch({ type: PLAY_TRACK, id }); | ||
break; | ||
default: | ||
// If we're not going to load this right away, | ||
// we should set duration on our own | ||
if (duration != null) { | ||
dispatch({ type: SET_MEDIA_DURATION, duration, id }); | ||
} else { | ||
dispatch(fetchMediaDuration(canonicalUrl, id)); | ||
} | ||
} | ||
|
||
if (metaData != null) { | ||
|
@@ -236,14 +229,35 @@ export function fetchMediaTags(file, id) { | |
.then(data => { | ||
// There's more data here, but we don't have a use for it yet: | ||
// https://github.com/aadsm/jsmediatags#shortcuts | ||
const { artist, title, picture } = data.tags; | ||
const { artist, title, picture } = data.common; | ||
let albumArtUrl = null; | ||
if (picture) { | ||
const byteArray = new Uint8Array(picture.data); | ||
const blob = new Blob([byteArray], { type: picture.type }); | ||
if (picture && picture.length >= 1) { | ||
const byteArray = new Uint8Array(picture[0].data); | ||
const blob = new Blob([byteArray], { type: picture[0].format }); | ||
albumArtUrl = URL.createObjectURL(blob); | ||
} | ||
dispatch({ type: SET_MEDIA_TAGS, artist, title, albumArtUrl, id }); | ||
|
||
// If we're not going to load this right away, | ||
// we should set duration on our own | ||
if (data.format.duration != null) { | ||
dispatch({ | ||
type: SET_MEDIA_DURATION, | ||
duration: data.format.duration, | ||
id | ||
}); | ||
} else { | ||
// ToDo? dispatch(fetchMediaDuration(canonicalUrl, id)); | ||
} | ||
|
||
dispatch({ | ||
type: SET_MEDIA, | ||
kbps: Math.round(data.format.bitrate / 1000).toString(), | ||
khz: Math.round(data.format.sampleRate / 1000).toString(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So exciting! |
||
channels: data.format.channels, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't even think of this! This was another piece of data that I wasn't able to get from the browser. 👏 |
||
length: data.format.duration, | ||
id | ||
}); | ||
}) | ||
.catch(() => { | ||
dispatch({ type: MEDIA_TAG_REQUEST_FAILED, id }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should do a |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,117 +18,55 @@ if (config.audioUrl && !config.initialTracks) { | |
export const skinUrl = config.skinUrl === undefined ? null : config.skinUrl; | ||
export const initialTracks = config.initialTracks || [ | ||
{ | ||
metaData: { artist: "DJ Mike Llama", title: "Llama Whippin' Intro" }, | ||
url: llamaAudio, | ||
duration: 5.322286 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's actually keep these metadata/duration. Without them we would always have to load music-metadata and fetch (at least part of) each media file, and we can show the proper title/duration in the playlist immediately. With them, we can defer loading music-metadata until someone drags in an unknown track. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. It's done on an isolated commit (472c2c0) on purpose, let's revert that one, for the sake of easy testing, until we merge back this branch. |
||
url: llamaAudio | ||
}, | ||
{ | ||
url: | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Diablo_Swing_Orchestra_-_01_-_Heroines.mp3", | ||
duration: 322.612245, | ||
metaData: { | ||
title: "Heroines", | ||
artist: "Diablo Swing Orchestra" | ||
} | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Diablo_Swing_Orchestra_-_01_-_Heroines.mp3" | ||
}, | ||
{ | ||
url: | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Eclectek_-_02_-_We_Are_Going_To_Eclecfunk_Your_Ass.mp3", | ||
duration: 190.093061, | ||
metaData: { | ||
title: "We Are Going To Eclecfunk Your Ass", | ||
artist: "Eclectek" | ||
} | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Eclectek_-_02_-_We_Are_Going_To_Eclecfunk_Your_Ass.mp3" | ||
}, | ||
{ | ||
url: | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Auto-Pilot_-_03_-_Seventeen.mp3", | ||
duration: 214.622041, | ||
metaData: { | ||
title: "Seventeen", | ||
artist: "Auto-Pilot" | ||
} | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Auto-Pilot_-_03_-_Seventeen.mp3" | ||
}, | ||
{ | ||
url: | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Muha_-_04_-_Microphone.mp3", | ||
duration: 181.838367, | ||
metaData: { | ||
title: "Microphone", | ||
artist: "Muha" | ||
} | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Muha_-_04_-_Microphone.mp3" | ||
}, | ||
{ | ||
url: | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Just_Plain_Ant_-_05_-_Stumble.mp3", | ||
duration: 86.047347, | ||
metaData: { | ||
title: "Stumble", | ||
artist: "Just Plain Ant" | ||
} | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Just_Plain_Ant_-_05_-_Stumble.mp3" | ||
}, | ||
{ | ||
url: | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Sleaze_-_06_-_God_Damn.mp3", | ||
duration: 226.795102, | ||
metaData: { | ||
title: "God Damn", | ||
artist: "Sleaze" | ||
} | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Sleaze_-_06_-_God_Damn.mp3" | ||
}, | ||
{ | ||
url: | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Juanitos_-_07_-_Hola_Hola_Bossa_Nova.mp3", | ||
duration: 207.072653, | ||
metaData: { | ||
title: "Hola Hola Bossa Nova", | ||
artist: "Juanitos" | ||
} | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Juanitos_-_07_-_Hola_Hola_Bossa_Nova.mp3" | ||
}, | ||
{ | ||
url: | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Entertainment_for_the_Braindead_-_08_-_Resolutions_Chris_Summer_Remix.mp3", | ||
duration: 314.331429, | ||
metaData: { | ||
title: "Resolutions (Chris Summer Remix)", | ||
artist: "Entertainment for the Braindead" | ||
} | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Entertainment_for_the_Braindead_-_08_-_Resolutions_Chris_Summer_Remix.mp3" | ||
}, | ||
{ | ||
url: | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Nobara_Hayakawa_-_09_-_Trail.mp3", | ||
duration: 204.042449, | ||
metaData: { | ||
title: "Trail", | ||
artist: "Nobara Hayakawa" | ||
} | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Nobara_Hayakawa_-_09_-_Trail.mp3" | ||
}, | ||
{ | ||
url: | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Paper_Navy_-_10_-_Tongue_Tied.mp3", | ||
duration: 201.116735, | ||
metaData: { | ||
title: "Tongue Tied", | ||
artist: "Paper Navy" | ||
} | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/Paper_Navy_-_10_-_Tongue_Tied.mp3" | ||
}, | ||
{ | ||
url: | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/60_Tigres_-_11_-_Garage.mp3", | ||
duration: 245.394286, | ||
metaData: { | ||
title: "Garage", | ||
artist: "60 Tigres" | ||
} | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/60_Tigres_-_11_-_Garage.mp3" | ||
}, | ||
{ | ||
url: | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/CM_aka_Creative_-_12_-_The_Cycle_Featuring_Mista_Mista.mp3", | ||
duration: 221.44, | ||
metaData: { | ||
title: "The Cycle (Featuring Mista Mista)", | ||
artist: "CM aka Creative" | ||
} | ||
"https://cdn.rawgit.com/captbaritone/webamp-music/4b556fbf/CM_aka_Creative_-_12_-_The_Cycle_Featuring_Mista_Mista.mp3" | ||
} | ||
]; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,26 @@ | ||
import invariant from "invariant"; | ||
import readStream from "filereader-stream"; | ||
|
||
import http from "stream-http"; | ||
|
||
function sourceToStream(source) { | ||
if (typeof source === "string") { | ||
// Assume URL | ||
return new Promise(resolve => { | ||
http.get(source, stream => { | ||
resolve({ | ||
stream, | ||
type: stream.headers["content-type"] | ||
}); | ||
}); | ||
}); | ||
} | ||
// Assume Blob | ||
return Promise.resolve({ | ||
stream: readStream(source), | ||
type: source.name | ||
}); | ||
} | ||
|
||
export function genMediaTags(file) { | ||
invariant( | ||
|
@@ -9,26 +31,22 @@ export function genMediaTags(file) { | |
if (typeof file === "string" && !/^[a-z]+:\/\//i.test(file)) { | ||
file = `${location.protocol}//${location.host}${location.pathname}${file}`; | ||
} | ||
return new Promise((resolve, reject) => { | ||
require.ensure( | ||
["jsmediatags/dist/jsmediatags"], | ||
require => { | ||
const jsmediatags = require("jsmediatags/dist/jsmediatags"); | ||
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); | ||
} | ||
}, | ||
() => { | ||
// The dependency failed to load | ||
}, | ||
"jsmediatags" | ||
); | ||
}); | ||
return require.ensure( | ||
["music-metadata"], | ||
require => { | ||
const mm = require("music-metadata"); | ||
return sourceToStream(file).then(stream => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The rest of the code base uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will adapt to your coding convention, no problem |
||
stream.type = stream.type ? stream.type.split(";")[0] : stream.type; // Strip off: ; charset=UTF-8 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure type will never be empty, and will always contain There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Empty or not containing In addition to that, f the type cannot be determined, I can add a fallback to use the actual data to determine the content type: Borewit/music-metadata#110 |
||
return mm.parseStream(stream.stream, stream.type, { duration: true }); | ||
}); | ||
}, | ||
err => { | ||
console.error("genMediaTags: Failed to load music-metadata"); | ||
// The dependency failed to load | ||
throw err; | ||
}, | ||
"music-metadata" | ||
); | ||
} | ||
|
||
export function genMediaDuration(url) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there some parsers which can't get duration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there are any, but there maybe specific case that is not possible. Anyway, probably good to make use of the expensive fall back scenario, using the audio element.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, and we need pass the size of the stream to the parser. The size is critical in some cases to calculate the duration.