Skip to content

Migrate music-metadata-browser to music-metadata #89

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Borewit
Copy link

@Borewit Borewit commented Jul 16, 2025

Changes:

  • Migrate music-metadata-browser to music-metadata.
  • Close web stream, when fetching metadata from URL
  • Pass Content-Type and Content-Length to music-metadata
  • Deprecate polyfills:
    • process
    • buffer
  • Adds ability to decode Ogg/FLAC radio streams

Related:

@Borewit Borewit force-pushed the migrate-to-music-metadata-dev branch from 88a7ff5 to 7899d16 Compare July 16, 2025 18:10
@hvianna
Copy link
Owner

hvianna commented Jul 16, 2025

@Borewit So I went to have a look at my previous migration attempt and remembered why I didn't go through with it..

If I understand it correctly, parseWebStream() requires that response.body is a readable byte stream, which is still unsupported on Safari (as per MDN documentation). It also requires at least Chromium 116, and I've been trying to keep audioMotion running on Chrome 87, because I use an old media box which is stuck on that version.

I don't know if it would be possible to have like a polyfill to provide compatibility, or if there is any alternative for parseWebStream() that wouldn't have this requirement.

image

@Borewit Borewit force-pushed the migrate-to-music-metadata-dev branch 5 times, most recently from d893428 to 8363512 Compare July 16, 2025 22:54
@Borewit
Copy link
Author

Borewit commented Jul 16, 2025

I think I have now handled the fallback for Safari. Yet I have no Safari to test with.

@Borewit Borewit force-pushed the migrate-to-music-metadata-dev branch 2 times, most recently from cc29bbe to 4a14286 Compare July 17, 2025 09:45
@Borewit
Copy link
Author

Borewit commented Jul 17, 2025

I think actually the default (non byte) web stream also works: Borewit/peek-readable#786

Note that module has been merged (with the fix) with https://github.com/Borewit/strtok3

Let me know if you see any remaining issue on your old media box @hvianna .

@Borewit Borewit force-pushed the migrate-to-music-metadata-dev branch 3 times, most recently from 6c7b159 to 6e763b0 Compare July 17, 2025 12:31
Copy link
Owner

@hvianna hvianna left a comment

Choose a reason for hiding this comment

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

Made some more observations. I hope to have time to test this more throughly over the weekend.

@Borewit Borewit force-pushed the migrate-to-music-metadata-dev branch 2 times, most recently from 6da3266 to 1c62fca Compare July 18, 2025 09:45
@Borewit Borewit requested a review from hvianna July 18, 2025 09:46
@Borewit Borewit force-pushed the migrate-to-music-metadata-dev branch 2 times, most recently from aba71c0 to 2326952 Compare July 18, 2025 09:55
@Borewit Borewit force-pushed the migrate-to-music-metadata-dev branch 2 times, most recently from dc4799c to 293c30e Compare July 18, 2025 19:23
@hvianna
Copy link
Owner

hvianna commented Jul 18, 2025

On web server mode it now reads a large part of the files to get the metadata and then stalls after some time. 😟 These are big 5.1 FLAC files ranging from ~50MB to ~500MB each. It seems to work fine on file system mode though.

BTW, my web parser function is not working with the directory listings generated by webpack-dev-server, so you may need to use npm start to test this.

image

@Borewit
Copy link
Author

Borewit commented Jul 19, 2025

On web server mode it now reads a large part of the files to get the metadata and then stalls after some time. 😟 These are big 5.1 FLAC files ranging from ~50MB to ~500MB each. It seems to work fine on file system mode though.

BTW, my web parser function is not working with the directory listings generated by webpack-dev-server, so you may need to use npm start to test this.

I like to load a portion of my library (I have more then 1 TB) into the queue, but how do I move /drag multiple folders into the playlist queue?

I have fairly large playlist as well, and I noticed some tracks did not resolved (dev branch). Not sure if this is caused by the obscure fetch implementation or of it has something to do with file encoding.

@hvianna
Copy link
Owner

hvianna commented Jul 19, 2025

I like to load a portion of my library (I have more then 1 TB) into the queue, but how do I move /drag multiple folders into the playlist queue?

Adding folders is not supported. You'll need to enter each folder and add all files (double arrow button).

@Borewit Borewit force-pushed the migrate-to-music-metadata-dev branch 2 times, most recently from 52a702e to 940a789 Compare July 19, 2025 14:57
@Borewit
Copy link
Author

Borewit commented Jul 19, 2025

I did a test, using the server fastify implementation #101, using a script running in Node.js traversing my music library.
Where the files are stored on NAS.

import {parseWebStream} from 'music-metadata';

const rootFolder = 'http://localhost:8080/music';

let numberOfFilesScanned = 0;

function increaseCounter() {
  ++numberOfFilesScanned;
  if (numberOfFilesScanned % 100 === 0) {
    const delta = performance.now() - startTime;
    const timePerFile = Math.round(100 * delta / numberOfFilesScanned) / 100;
    console.log(`Scanned ${numberOfFilesScanned} audio files, in ${timePerFile} ms per file`);
  }
}

async function scanFolder(folder) {
  // console.log(`Scanning folder: ${folder}`);
  const response = await fetch(`${folder}/`);
  if (response.ok) {
    const folderListing = await response.json();
    for (const file of folderListing.files) {
      if (isAudioFile(file.name)) {
        const url = `${folder}/${encodeURIComponent(file.name)}`;
        // console.log(`Fetching ${url}...`);
        const response = await fetch(url);
        if (response.ok && response.body) {
          const mm = await parseWebStream(response.body);
          // await response.body.cancel();
          increaseCounter();
        }
      }
    }
    for (const subFolder of folderListing.dirs) {
      await scanFolder(`${folder}/${encodeURIComponent(subFolder.name)}`); // Recursion
    }
  }

}

function isAudioFile(filename) {
  const fileExt = filename.split('.').pop()?.toLowerCase();
  return ['flac', 'mp3', 'wav', 'm4a', 'opus'].indexOf(fileExt) !== -1;
}

const startTime = performance.now();
scanFolder(rootFolder);

Which outputs:

Scanned 100 audio files, in 45.31 ms per file
Scanned 200 audio files, in 46.18 ms per file
Scanned 300 audio files, in 45.79 ms per file
Scanned 400 audio files, in 45.22 ms per file
Scanned 500 audio files, in 46.6 ms per file
Scanned 600 audio files, in 50.2 ms per file
Scanned 700 audio files, in 51.1 ms per file
Scanned 800 audio files, in 51.32 ms per file
Scanned 900 audio files, in 50.32 ms per file
Scanned 1000 audio files, in 49.85 ms per file

These are all FLAC files. So it scans a FLAC file 50 ms, I think that performance is not to bad.

@Borewit Borewit force-pushed the migrate-to-music-metadata-dev branch 3 times, most recently from ba9c579 to dd71b92 Compare July 20, 2025 11:58
@Borewit
Copy link
Author

Borewit commented Jul 20, 2025

On web server mode it now reads a large part of the files to get the metadata and then stalls after some time. 😟 These are big 5.1 FLAC files ranging from ~50MB to ~500MB each. It seems to work fine on file system mode though.

BTW, my web parser function is not working with the directory listings generated by webpack-dev-server, so you may need to use npm start to test this.

image

Nailed the degradation in performance here: #103

@Borewit Borewit force-pushed the migrate-to-music-metadata-dev branch from dd71b92 to 80ce4ee Compare July 20, 2025 13:43
@Borewit
Copy link
Author

Borewit commented Jul 20, 2025

Restored mostly the original metadata load limitation mechanism, as I made things worse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants