Skip to content
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

Simplify API and fix a few issues #41

Open
nikhilm opened this issue Apr 27, 2013 · 3 comments
Open

Simplify API and fix a few issues #41

nikhilm opened this issue Apr 27, 2013 · 3 comments
Assignees

Comments

@nikhilm
Copy link
Owner

nikhilm commented Apr 27, 2013

I've pushed a new README.md to branch 'memtag', which describes how the new API would be.

The Metadata object is actually a standard JS object with none of the accessor code that is currently in use for the Tag object. It has write() and writeSync() methods which will handle copying back values from v8 to taglib.

This simplifies the API drastically - no read() vs. tag(), no 'audio-properties are second class'. There is also a big reduction in the number of lines of code under the hood.

The changes will also fix parts of #38 which is that FileRefs will be closed as soon as data has been read, and then save() will reopen a fileref.
#35 will also be fixed with readSync() providing access to audio properties as well.

I have some WIP patches that make all these changes. @lennart @masterkain I would appreciate your thoughts on this. Thanks!

@ghost ghost assigned nikhilm Apr 27, 2013
@masterkain
Copy link
Contributor

Sounds good to me, we haven't completed the switch to node 0.10 yet but it's in the pipeline and we can help testing.

Thanks 👍

@lennart
Copy link
Contributor

lennart commented Apr 29, 2013

I really appreciate the more consistent proposed API, haven't been upgrading to 0.10 either though.

I would love to see the changes for FileRef handling, as the windows port of a project is stuck since repeated calls to the same file fail with the current version.

@nikhilm
Copy link
Owner Author

nikhilm commented Apr 30, 2013

I've pushed the patches as one big commit (sorry).

AFAIK there are no hard dependencies on node v0.10, likely only minor compile and typedef errors. I don't have a ready build on 0.8 right now and I'm really tired, but if someone could comment with any errors, that'd be great!

@lennart could you check the fileref handling issue on windows? On Linux, if I try to read a large number of files, I still get some errors, possibly because I'm leaking something or maybe v8/libuv aren't playing well together at getting rid of FileRef's faster. I need to investigate this.

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

No branches or pull requests

3 participants