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

Integration with music-metadata #606

Closed
wants to merge 43 commits into from
Closed
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
475f1f6
Replace jsmediatags with music-metadata to read metadata from audio t…
Borewit Jun 19, 2018
e7fd0b4
Excluding graceful.js no longer needed, removed dependency from music…
Borewit Jun 19, 2018
646bd11
Use webpack.IgnorePlugin to ignore fs module.
Borewit Jun 21, 2018
33673a6
Fix fs-extra dependency in index (main)
Borewit Jun 21, 2018
c8be1ff
Ignore module fs.
Borewit Jun 21, 2018
800e156
Be able to stream from a provided URL location.
Borewit Jun 21, 2018
c988ef8
Fix music-metadata dependency
Borewit Jun 22, 2018
37c4273
Strips of the: `; charset=UTF-8` (which should only be used for `text…
Borewit Jun 22, 2018
e364610
Use music-metadata to resolve the duration.
Borewit Jun 22, 2018
472c2c0
Remove predefined duration & metadata from initial play-list.
Borewit Jun 22, 2018
963e0ee
Merge branch 'master' into music-metadata
Borewit Jun 22, 2018
aff1c18
Merge remote-tracking branch 'upstream/master' into music-metadata
Borewit Jun 22, 2018
27241ba
Re-enable Buffer
Borewit Jun 22, 2018
f21cf3a
Use music-metadata to resolve the bit-rate and sample-rate
Borewit Jun 22, 2018
d086326
Avoid inclusion of blue-bird, file-type & debug to be included in mus…
Borewit Jun 23, 2018
acac8bc
Use music-metadata dependency to ^2.1.0
Borewit Jun 23, 2018
ae13e55
Update to music-metadata 2.2.0 (without bluebird module) to save space.
Borewit Jun 24, 2018
2c6270f
Merge branch 'master' of https://github.com/captbaritone/webamp into …
Borewit Jun 24, 2018
a6ac1fd
Merge branch 'master' into music-metadata
Borewit Jun 25, 2018
d56acd7
Use async/await conform code style
Borewit Jun 26, 2018
d927b3d
Update music-metadata dependency to v2.3.0.
Borewit Jul 4, 2018
9e137c6
Update music-metadata dependency to 2.3.1
Borewit Jul 6, 2018
25ba886
Merge with master branch.
Borewit Jul 6, 2018
edfdcaf
Align dependencies with master to prevent unit test from failing
Borewit Jul 7, 2018
7d72395
Merge branch 'master' into music-metadata
Borewit Jul 14, 2018
4cee6ae
Merge branch 'master' into music-metadata
Borewit Jul 16, 2018
8159a13
Update music-metadata: calculate bit-rate FLAC files.
Borewit Jul 16, 2018
1005a4d
Pass file-size blob to music-metadata
Borewit Jul 16, 2018
7fd8740
Update to music-metadata 2.4.0: enabling lazy loading
Borewit Jul 17, 2018
a4e240c
Merge branch 'master' into music-metadata
Borewit Jul 19, 2018
ba389e3
#606 Update music-metadata which adds flag to avoid ID3v1 headers are…
Borewit Jul 19, 2018
d210971
Revert "Remove predefined duration & metadata from initial play-list."
Borewit Jul 19, 2018
032a770
Make WebPack ignore TypeScript definition (.d.ts) files.
Borewit Jul 28, 2018
cff4858
Update music-metadata dependency
Borewit Jul 28, 2018
b06c8b7
Exclude typescript definition file. Pending issue in UglifyJS.
Borewit Jul 28, 2018
5f1fbe7
Ignore TypeScript definition using ignore-loader.
Borewit Jul 28, 2018
1070c15
Move ignore-loader to dev dependencies
Borewit Jul 28, 2018
3a2e5ae
Update to music-metadata 2.5.0
Borewit Aug 16, 2018
b52b46e
Merge branch 'master' into music-metadata
Borewit Aug 17, 2018
fc5bcb6
Revert "Ignore TypeScript definition using ignore-loader."
Borewit Aug 17, 2018
6bfde63
Revert "Move ignore-loader to dev dependencies"
Borewit Aug 17, 2018
ba217b8
Update music-metadata dependency to 2.6.0
Borewit Aug 17, 2018
f3d165a
Replace music-metadata with music-metadata-browser dependency
Borewit Aug 26, 2018
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
6 changes: 3 additions & 3 deletions config/webpack.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@ const path = require("path");
const HtmlWebpackPlugin = require("html-webpack-plugin");
const WebpackPwaManifest = require("webpack-pwa-manifest");
const HtmlWebpackInlineSVGPlugin = require("html-webpack-inline-svg-plugin");
const webpack = require("webpack");

module.exports = {
resolve: {
extensions: [".js"]
},
node: {
// Consider suggesting jsmediatags use: https://github.com/feross/is-buffer
// Cuts 22k
Buffer: false
fs: "empty" // Ignore fs in music-metadata
},
module: {
rules: [
Expand Down Expand Up @@ -41,6 +40,7 @@ module.exports = {
noParse: [/jszip\.js$/]
},
plugins: [
new webpack.IgnorePlugin(/fs/, /file-type/, /debug/), // Ignore fs in music-metadata
new HtmlWebpackPlugin({
template: "./index.html"
}),
Expand Down
1 change: 1 addition & 0 deletions config/webpack.library.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ module.exports = {
NODE_ENV: JSON.stringify("production")
}
}),
new webpack.IgnorePlugin(/fs/),
new UglifyJsPlugin({
include: /\.min\.js$/,
parallel: true,
Expand Down
42 changes: 28 additions & 14 deletions js/actionCreators/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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));
Copy link
Owner

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

}

dispatch({
type: SET_MEDIA,
kbps: Math.round(data.format.bitrate / 1000).toString(),
khz: Math.round(data.format.sampleRate / 1000).toString(),
Copy link
Owner

Choose a reason for hiding this comment

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

So exciting!

channels: data.format.channels,
Copy link
Owner

Choose a reason for hiding this comment

The 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 });
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we should do a fetchMediaDuration here?

Expand Down
88 changes: 13 additions & 75 deletions js/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Owner

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

@Borewit Borewit Jun 26, 2018

Choose a reason for hiding this comment

The 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"
}
];

Expand Down
58 changes: 38 additions & 20 deletions js/fileUtils.js
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(
Expand All @@ -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 => {
Copy link
Owner

Choose a reason for hiding this comment

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

The rest of the code base uses async/await. Might as well use it here too.

Copy link
Contributor Author

@Borewit Borewit Jun 26, 2018

Choose a reason for hiding this comment

The 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
Copy link
Owner

Choose a reason for hiding this comment

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

Are we sure type will never be empty, and will always contain ;?

Copy link
Contributor Author

@Borewit Borewit Jun 26, 2018

Choose a reason for hiding this comment

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

Empty or not containing ; is not issue I believe, undefined or null is different story.
I will consider this to move this to music-metadata.

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) {
Expand Down
8 changes: 4 additions & 4 deletions js/mediaMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
SEEK_TO_PERCENT_COMPLETE,
SET_BAND_VALUE,
SET_BALANCE,
SET_MEDIA,
SET_VOLUME,
START_WORKING,
STOP,
Expand All @@ -19,7 +18,6 @@ import {
CHANNEL_COUNT_CHANGED
} from "./actionTypes";
import { next as nextTrack } from "./actionCreators";
import { getCurrentTrackId } from "./selectors";

export default media => store => {
const {
Expand Down Expand Up @@ -55,16 +53,18 @@ export default media => store => {
store.dispatch({ type: STOP_WORKING });
});

// ToDo: maybe as an alternative if music-metadata did managed to read all
/*
media.on("fileLoaded", () => {
store.dispatch({
type: SET_MEDIA,
kbps: "128",
//kbps: "128",
khz: Math.round(media.sampleRate() / 1000).toString(),
channels: media.channels(),
length: media.duration(),
id: getCurrentTrackId(store.getState())
});
});
}); */

media.on("channelupdate", () => {
store.dispatch({
Expand Down
7 changes: 5 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
"eslint-plugin-prettier": "^2.2.0",
"eslint-plugin-react": "^7.7.0",
"file-loader": "^1.1.5",
"filereader-stream": "^2.0.0",
"git-revision-webpack-plugin": "^2.5.1",
"gzip-size-cli": "^2.0.0",
"html-webpack-inline-svg-plugin": "^1.2.4",
Expand All @@ -79,8 +80,8 @@
"jest": "^23.0.0",
"jest-mock-random": "^1.0.2",
"jest-runner-eslint": "^0.4.0",
"jsmediatags": "^3.8.1",
"jszip": "^3.1.3",
"music-metadata": "^2.2.0",
"prettier": "^1.13.0",
"prop-types": "^15.5.10",
"puppeteer": "^1.4.0",
Expand All @@ -96,6 +97,7 @@
"redux-thunk": "^2.1.0",
"reselect": "^3.0.1",
"screenfull": "^3.3.2",
"stream-http": "^2.8.3",
"style-loader": "^0.19.1",
"tinyqueue": "^1.2.3",
"travis-weigh-in": "^1.0.2",
Expand All @@ -115,5 +117,6 @@
"projects": [
"config/jest.*.js"
]
}
},
"dependencies": {}
}
Loading