DEV Community

Hammad Shams Uddin
Hammad Shams Uddin

Posted on

I made twenty tools take twenty files. Every bug was state that outlived one file.

My browser-based tools each took exactly one file. Converting twenty tracks meant twenty page loads, and on the background remover each of those re-downloaded a 44 MB segmentation model to do three seconds of work.

So I put a loop around the work. That is genuinely most of it — read the settings once, run them over each file, collect the results, zip them.

Then I tested it in a real browser and found three bugs. They look unrelated. They are the same bug: something that was fine as a single value became wrong the moment there were two files, and none of them announced itself.

Bug one: the second file overwrote the first

The single-file version wrote into a fixed name on ffmpeg.wasm's virtual filesystem:

function runSingle(o) {
  const inN  = 'in.'  + ext(files[0].name);
  const outN = 'out.' + pick.ext;
  // ...
}
Enter fullscreen mode Exit fullscreen mode

That filesystem is shared for the whole session, not per call. Loop it, and clip two writes in.mp4 over clip one — which, if you are unlucky with timing, is not even a crash. It is a correct-looking output that contains the wrong video.

The fix is boring: the index goes in the name.

function runSingle(o, file, idx) {
  const inN  = 'in'  + idx + '.' + ext(file.name);
  const outN = 'out' + idx + '.' + pick.ext;
Enter fullscreen mode Exit fullscreen mode

What is not boring is the second half. A file that fails leaves its input behind, and the next twelve files then run in a workspace that is filling up with dead inputs. So the cleanup has to happen on the failure path too, not just the success one:

  }).then(function (data) {
    drop(inN); drop(outN);
    return data;
  }, function (e) {
    drop(inN); drop(outN);   // <- the one people forget
    throw e;
  });
Enter fullscreen mode Exit fullscreen mode

Bug two: the array that was empty for the whole run

This is the one worth the article.

Two output files can want the same name. song.wav and song.flac both convert to song.mp3, and a zip quietly keeps only the last of a repeated name — so one of the two conversions silently vanishes. I wrote a dedupe for it:

const out = {
  name: uniqueName(stem + '.png', outputs),   // compare against what we have so far
  blob: blob
};
Enter fullscreen mode Exit fullscreen mode

And a preview, so a batch shows a picture rather than only a list of names:

if (!outputs.length) { showPreview(blob); }   // only the first one
Enter fullscreen mode Exit fullscreen mode

Both read outputs. And outputs is assigned like this:

series(files, job).then(function (res) {
  outputs = res.done;      // <- after every file has finished
  render(outputs);
});
Enter fullscreen mode Exit fullscreen mode

So for the entire duration of the run — the only time those two lines execute — outputs is [].

The consequences are not symmetrical, which is why this took a test to catch:

  • !outputs.length was always true, so every file overwrote the preview. The batch appeared to work and showed the last image. Nothing looked broken; the preview was just the wrong file, and only if you knew which one you dropped first would you notice.
  • uniqueName(want, []) never found a clash, so cat.png and cat.jpg both produced cat-no-bg.png and the zip contained one of them. The results list on screen showed the name twice, which is the only reason I looked.

The fix is one line and it is not the dedupe:

const made = [];                       // accumulate where the work happens
series(files, function (f, i) {
  return job(f, i).then(function (out) {
    const o = { name: uniqueName(stem, made), blob: out };
    if (!made.length) { showPreview(out); }
    made.push(o);
    return o;
  });
}).then(function (res) { outputs = res.done; });
Enter fullscreen mode Exit fullscreen mode

The general shape:

A variable assigned from a promise's result is empty for exactly as long as the work that produces it is running. If anything inside that work reads it, it reads the empty version, every time.

It is easy to miss because the code reads correctly in the order it is written. outputs = res.done sits above render(outputs) and below the loop, so it looks like it happens between them. It happens after both.

Bug three: an operation completing is not evidence it did anything

I have written about this one before, and batching brings it back sharper. ffmpeg.run() resolves whether ffmpeg succeeded or not. With one file, a silent failure means the visitor sees an error a moment later and tries again. With twenty, one silent failure means nineteen good files and one wrong one, in a zip, unremarked.

So each iteration has to judge the output, not the promise:

const data = ffmpeg.FS('readFile', outN);
if (!data || !data.length) {
  throw new Error('ffmpeg wrote no output for ' + file.name);
}
Enter fullscreen mode Exit fullscreen mode

And the failure has to be per-file rather than fatal. One unreadable file out of twenty must not cost someone the other nineteen — it should be named on the results page, next to the files that worked:

alpha.mp3          7 KB   Save
song.mp3           6 KB   Save
song (2).mp3       5 KB   Save
1 file could not be processed and is not included. broken.wav
Enter fullscreen mode Exit fullscreen mode

Sequential, not Promise.all

Worth saying because it looks like the obvious optimisation. There is one ffmpeg instance behind one core and one ONNX session. Running the files concurrently does not make them finish sooner; it interleaves them, multiplies peak memory by the number of files, and turns a readable progress bar into a lie.

The batch is a reduce over promises, and the bar gives each file an equal slice:

function setProgress(r) {
  const p = Math.round(((done + clamp01(r)) / total) * 100);
  bar.style.width = p + '%';
}
Enter fullscreen mode Exit fullscreen mode

How I actually found these

None of the three would have been caught by reading the diff, and two of them produce a page that looks like it worked.

They came out of driving the shipped page in a real browser: build the fixtures in JavaScript, hand them to the widget as a synthetic DataTransfer, click the button, and read the DOM afterwards. The fixtures are the part that matters — four files where two deliberately fight over one output name and one is eight bytes of garbage:

dt.items.add(file('alpha.wav', wav(0.5, 440)));
dt.items.add(file('song.wav',  wav(0.4, 300)));
dt.items.add(file('song.m4a',  wav(0.3, 600)));   // same stem -> collision
dt.items.add(file('broken.wav', new Uint8Array([1,2,3,4,5,6,7,8]).buffer));
Enter fullscreen mode Exit fullscreen mode

A happy-path batch of three well-named valid files passes with all three bugs present. That is not a test; it is a screenshot with extra steps.


I build Utilorax, a set of free browser-based tools. The batch now runs on the audio converter, the video converter and the background remover — nothing is uploaded, so the loop and the failures are all on your own machine.

Top comments (0)