interbred-monkey / id3_reader

A Node.js ID3 Tag reader
15 stars 6 forks source link

parallel calls #5

Closed benkaiser closed 8 years ago

benkaiser commented 9 years ago

When scanning a set of files by issuing a bunch of calls (say 10), I have only the first 1 return a value and the other 9 return an error "Unable to process file" .

Then when putting the calls within an asycn.eachSeries (making them all execute in-order) it works just fine. Can this library be modified to work in parallel?

Thanks for your awesome work @interbred-monkey :+1:

benkaiser commented 9 years ago

I've isolated the issue. It's to do with how the require('./includes/tag_reader.js') works.

When the tagreader is required and stored in tagReader, that module is only loaded once, and used on all future occasions. So when you do new tagReader(file, function(err, tag_buffer) { it is creating a new instance of tagReader, and then inside the module it is assigning it to _instance. Now the only issue is that _instance is shared by every tagReader that is created, so the functions start overlapping each other, and then closeFile is called several times on a file that doesn't exist causing the error.

Now the solution to this is to make instance only scope to when a new tagReader is created. This can be done by wrapping all the function in tag_reader.js like so:

var tagReaderOuter = function(params, callback) {
  var _instance = null;

  //
  // all the tagreader functions
  //

  new tagReader(params, callback);
}

module.exports = tagReaderOuter;

You can make the solution more elegant if you wish, but I'll leave it up to you. Also when this is fixed can you update the npm package?

interbred-monkey commented 9 years ago

Hey dude, I am gonna have a look at this soon I just need to get my laptop back first.

If I have not implemented a fix after a few weeks don't let me forget.

Thanks

benkaiser commented 9 years ago

All good @simonmudd I'm not in a rush. I ended up using a different solution that depended on ffmpeg to handle some basic id3 tag writing functionality (for now), however it only covers a small set of the attributes (title, artist, album, year, genre and album art). It would be really nice to have a native node module like id3_reader that could deal with the rest of the attributes.

rtlong commented 9 years ago

@benkaiser's fix #7 fixes the issue for me :+1: Thanks!

interbred-monkey commented 8 years ago

This issue should now be resolved via a merge from @9jaboy, sorry for the delay in this update.