TooTallNate / node-ogg

Node.js native binding to libogg
MIT License
65 stars 36 forks source link

Port to use NAN #7

Closed Xenoveritas closed 9 years ago

Xenoveritas commented 9 years ago

Tests pass and this appears to work in my actual code under 64-bit Windows with VS2013.

Rantanen commented 9 years ago

Any chance to get this merged and to npm, @TooTallNate? Most Opus files/streams use ogg containers as well, which is expanding the need for ogg encoder/decoder beyond the usual vorbis/theora.

Tested this on Node v0.12.2 in Linux x64 and it seems to work (well.. as far as the tests cover it anyway)

@Xenoveritas, can you add Node 0.12 in the .travis.yml and appveyor.yml as well? Or can PRs even do that to get Travis/Appveyor to run the build against those?

The failing appveyor case seems to be something unrelated:

npm install
'npm' is not recognized as an internal or external command,
operable program or batch file.

Is there anything that is still missing and could be done to get this in quicker?

TooTallNate commented 9 years ago

Getting a compilation warning with clang:

../src/binding.cc:50:43: warning: field 'rtn' will be initialized after base 'NanAsyncWorker' [-Wreorder]
    : oy(oy), buffer(buffer), size(size), rtn(0), NanAsyncWorker(callback) { }
                                          ^
../src/binding.cc:87:15: warning: field 'rtn' will be initialized after field 'serialno' [-Wreorder]
    : oy(oy), rtn(0), serialno(-1), packets(-1), page(page), NanAsyncWorker(callback)
              ^
../src/binding.cc:87:37: warning: field 'packets' will be initialized after field 'page' [-Wreorder]
    : oy(oy), rtn(0), serialno(-1), packets(-1), page(page), NanAsyncWorker(callback)
                                    ^
../src/binding.cc:87:50: warning: field 'page' will be initialized after base 'NanAsyncWorker' [-Wreorder]
    : oy(oy), rtn(0), serialno(-1), packets(-1), page(page), NanAsyncWorker(callback)
                                                 ^
../src/binding.cc:139:27: warning: field 'rtn' will be initialized after base 'NanAsyncWorker' [-Wreorder]
    : os(os), page(page), rtn(0), NanAsyncWorker(callback) { }
                          ^
../src/binding.cc:172:31: warning: field 'rtn' will be initialized after base 'NanAsyncWorker' [-Wreorder]
    : os(os), packet(packet), rtn(0), NanAsyncWorker(callback) { }
                              ^
../src/binding.cc:220:31: warning: field 'rtn' will be initialized after base 'NanAsyncWorker' [-Wreorder]
    : os(os), packet(packet), rtn(0), NanAsyncWorker(callback) { }
                              ^
TooTallNate commented 9 years ago

Warnings fixed in 0037954452816f5f56b2f7ff765c9ebbaee997da.

v1.2.0 published to npm. Thanks guys!