Borewit / music-metadata-browser

Browser version of music-metadata parser Supporting a wide range of audio and tag formats.
MIT License
240 stars 21 forks source link

Multiple blobs at once not parsing in WebWorker in Firefox #948

Closed wvffle closed 4 months ago

wvffle commented 11 months ago

I've decided to move the parsing logic to the web worker, so that the main thread and ui wouldn't be impacted by analyzing lots of files.

Consider following web worker code:

/// <reference lib="webworker" />

import * as Metadata from 'music-metadata-browser'

const parse = async (id: number, file: File) => {
  try {
    console.log(`[${id}] parsing...`)
    const metadata = await Metadata.parseBlob(file)
    console.log(`[${id}] metadata:`, metadata)
    postMessage({ 
      id,
      status: 'success',
      metadata, 
      coverUrl 
    })
  } catch (error) {
    console.error(error)
    postMessage({ 
      id,
      status: 'failure',
      error
    })
  }
}

// ...

Now, when I try to parse it the easiest way possible, only a few random files are parsed:

addEventListener('message', async (event) => {
  const id = event.data.id as number
  const file = event.data.file as File
  parse(id, file)
})

image

In that case, only files with id 3 and 4 have been parsed. Note that pausing, the web worker thread in the devtools does nothing until I send a new file to process to the worker. It then breaks on the addEventListener. The parseBlob never resolves nor rejects.

Also, before moving all the logic into the web worker I could start parsing all files at the same time without any problems like so:

await Promise.allSettled(files.map(async (file) => {
  return Metadata.parseBlob(file)
}))

The workaround that I need to introduce now is to wait until one file is parsed and then start parsing the next one. That's not really what I want to do as it would be much less performant.

const queue = []
let queuePromise = Promise.resolve()
addEventListener('message', async (event) => {
  const id = event.data.id as number
  const file = event.data.file as File

  queue.push(() => parse(id, file))
  queuePromise = queuePromise.then(() => queue.shift()())
})

Affected browser: Firefox 120, it works well on Chromium

Vite 4 with the ngmi-polyfill plugin as per https://github.com/Borewit/music-metadata-browser/issues/836#issuecomment-1455011009

Borewit commented 11 months ago
await Promise.allSettled(files.map(async (file) => {
 return Metadata.parseBlob(file)
}))

Better to limit the number of parallel calls. You may use something like p-limit.

addEventListener('message', async (event) => {
  const id = event.data.id as number
  const file = event.data.file as File
  parse(id, file)
})

Looks like the promise returned by parse is ignored.

const queue = []
let queuePromise = Promise.resolve()
addEventListener('message', async (event) => {
 const id = event.data.id as number
 const file = event.data.file as File

 queue.push(() => parse(id, file))
 queuePromise = queuePromise.then(() => queue.shift()())
})

I fail to see the point this mechanism

wvffle commented 11 months ago

Better to limit the number of parallel calls. You may use something like p-limit.

That is a good idea, thanks. I will probably use it.

Looks like the promise returned by parse is ignored.

Awaiting this call would change nothing actually. Everything is handled inside the parse function. I will create a minimal reproduction repository tomorrow, so you could better understand the issue.

I fail to see the point this mechanism

This mechanism simply queues the parsing of each file so that only one file is parsed at once.

Edit: After some more testing, it seems that even when I run the parsing sequentially, it can just hang without ever resolving or rejecting parseBlob when in Firefox's Web Worker

wvffle commented 11 months ago

@Borewit, here I created a minimal reproduction: https://stackblitz.com/edit/vitejs-vite-mrd4bk?file=src%2Fmain.ts,src%2Fworker.ts&terminal=dev

From my testing, sometimes it gets stuck on parsing blob even if I send a single file while using Firefox 120. It works well in Chromium and in Firefox but outside Web Worker.

Borewit commented 11 months ago

This mechanism simply queues the parsing of each file so that only one file is parsed at once.

Smart, understood.

Borewit commented 11 months ago

I am travelling now for a few days, I get back to you after.

Borewit commented 4 months ago

Closing this issue, you give music-metadata, I will deprecate this package.