minht11 / local-music-pwa

Lightweight on device music player PWA.
https://snaeplayer.com/
MIT License
154 stars 20 forks source link

Try music-metadata-browser? #5

Closed Borewit closed 3 years ago

Borewit commented 5 years ago

Would you like to try music-metadata-browser?

It supports about any audio file around, promises, TypeScript, duration.

I failed to get the project running, I only tested this PR up to the point where it compiles.

I look forward you your feedback @minht11.

minht11 commented 3 years ago

I am very sorry it took so long for me to respond. At the very beginning I have tried using music-metadata, but since it was published as CJS module(not sure if it still is), it would incorrectly compile inside Worker file. The package I use right now also had similar issues, as a workaround I imported ES6 source files instead of published modules, I am not sure if that would work here as well. Another thing that concerns me is size: music-metadata is big package with a lot of functionality, right now on bundlephobia it says that minified version is bigger than my whole app. That isn't a deal breaker in on itself because it would be bundled separately from the main app inside the worker file. Right now I only support mp3 files so additional file format support would be great, so I will probably look into this.

lennyanders commented 3 years ago

Hey, if you're still interested in implementing music-metadata-browser, I think I got it working in a worker.

So I basically added this to the top of the worker to polyfill buffer:

// polyfill buffer for music-metadata-browser
// @ts-ignore
import { Buffer } from 'safer-buffer';
// @ts-ignore
globalThis.Buffer = Buffer;

And then added this to my vite config, so music-metadata-browser accesses globalThis and not global:

define: {
  global: 'globalThis',
},

I hope this helps. If you want, I can try to implement this in the next few days into this project and make a PR.

Some off-topic stuff: So I was working on a music player in electron, because I "always" wanted my own music player. I didn't really like the idea of using electron, but I also didn't want to learn a completely new programming language and completely new programming paradigms. Yesterday I searched again if it was possible to build a music player for local files, only using the/a browser and finally found this project on Reddit, and it helped me already a lot. If my own thing should at some time be in a usable state, I will definitely name your project as an Inspiration/help.

Borewit commented 3 years ago

I am not sure safe-buffer has all the required functionality.

I have done it like this:

import * as process from 'process';
(window as any).process = process;

(window as any).global = window;

import * as _buffer from 'buffer';
(window as any).Buffer = _buffer.Buffer; // note: the trailing slash is important!

Ref: https://github.com/Borewit/audio-tag-analyzer/blob/899f5995d5b8e3a0aa5362022fae441644ddf7fb/src/polyfills.ts#L37-L43

ES Modules are now support by Node.js, I will soon look into migrating music-metadata, ref: Borewit/music-metadata#885.

lennyanders commented 3 years ago

Yeah, not sure why I choose safer-buffer, buffer seems to be the better package and is already a dependency of music-metadata-browser, so it makes even less sense. We can't add things to the window because we live in a Web Worker, and I guess the process import is probably Angular specific.

minht11 commented 3 years ago

@lennyanders yeah I would really appreciate if you could make a PR for this.

Latest music-metadata seems to support Uint8Array as parseBuffer argument, so we can use 'music-metadata/lib/core' directly. If you add polyfills for built in node modules, process and buffer, like shown previously to globalThis instead of window, I think it should work.

Borewit commented 3 years ago

I believe process is required in one of the sub dependencies, I thought readable-stream, but I am note sure.

These are dependencies of https://npmgraph.js.org/?q=music-metadata-browser

Latest music-metadata seems to support Uint8Array as parseBuffer argument

Yet internally there are still dependency on Node.js like Buffer object, as it there are functions to decode text which are not supported by Uint8Array either by DataView.

lennyanders commented 3 years ago

I got it to work there with minimal changes: #6

minht11 commented 3 years ago

Fixed in https://github.com/minht11/local-music-pwa/pull/6 Thanks