TheLinx / lao

A library for audio output through Lua.
http://thelinx.github.com/lao/
18 stars 2 forks source link

Fixes #4

Closed daurnimator closed 7 years ago

daurnimator commented 7 years ago
daurnimator commented 7 years ago

For @peterbillam

peterbillam commented 7 years ago

Looks good :-) but

Compiling "Don't bother closing when NULL. Don't push just to throw away" I get: src/lao.c: In function ‘l_close_device’: src/lao.c:193:6: error: dereferencing pointer to incomplete type ‘ao_device {aka struct ao_device}’ if (*dev != NULL) ^~~~ Makefile:22: recipe for target 'ao.so' failed

Apart from that, there's just the usual warnings in l_initialize l_shutdown and l___gc about warning: unused parameter ‘L’ [-Wunused-parameter] static int l_shutdown(lua_State* L) and I guess there's a case for leaving those L in, just like you can put a trailing comma after the last item in a table ...

daurnimator commented 7 years ago

@peterbillam fixed.

TheLinx commented 7 years ago

Hello, Thank you for the contributions. I myself haven't touched Lua in years now so I'm not the best maintainer for this. I'm not sure on the list of things I'd have to do to shift responsibility but if you want it, I can go ahead and give it to you instead.

peterbillam commented 7 years ago

Yes, nice :-) And good to hear from TheLinx.

I had to add to example* the compatibility-lines local numeric_version = string.gsub(_VERSION, "^%D+", "") if tonumber(numeric_version) < 5.2 then bit = require 'bit' -- LuaBitOp http://bitop.luajit.org/api.html elseif _G.bit32 then bit = _G.bit32 else local f = load([[ bit.bor = function (a,b) return a|b end bit.band = function (a,b) return a&b end bit.rshift = function (a,n) return a>>n end ]]) f() end

And also I notice that in play() the length of the byte-string is greater than buf_size eg: buf_size = 176400 #buffer = 176404 str.len = 176404 so that if I use either local rv = device:play(str) or local rv = device:play(str, string.len(str)) I get the message lua: pjb_ao_test.lua:106: bad argument #2 to 'play' (too many bytes) stack traceback: [C]: in function 'play' pjb_ao_test.lua:106: in main chunk

but it beeps sweetlly sinusoidally :-)

daurnimator commented 7 years ago

And also I notice that in play() the length of the byte-string is greater than buf_size eg:

Oops; off by one error. fixed.

daurnimator commented 7 years ago

Thank you for the contributions. I myself haven't touched Lua in years now so I'm not the best maintainer for this. I'm not sure on the list of things I'd have to do to shift responsibility but if you want it, I can go ahead and give it to you instead.

I don't particularly want to take over the library: I have plenty to maintain already, and I don't use lao myself.

If you just want to add me as a collaborator here on github that's fine: that way I can merge things myself if I want to.

peterbillam commented 7 years ago

Well, I do plan to use lao a bit, but my big disadvantage is that I'm 69, and last year had a very dangerous medical emergency and am not feeling nearly as immortal as I used to, so you'd probably be looking for a replacement quite soon.. I guess taking over mainly involves tagging it when it's stable, and being in charge of the rockspec. So I'd like it if another volunteer could be found, and another may have to be found soon anyway. If daurnimator could be made a collaborator that would be great, it's wonderful how he's got ao working.

peterbillam commented 7 years ago

Whenever I require "ao" I get fifteen lines of debug messages like debug: Loaded driver null (built-in) debug: Loaded driver wav (built-in) and so on. It comes from libao.so Is there a way of turning debug off by default ? I couldn't see it in https://xiph.org/ao/doc/ ...

peterbillam commented 7 years ago

debug off is easy :-) See https://xiph.org/ao/doc/config.html In /etc/libao.conf or ~/.libao, set quiet=yes Though the debug and verbose options didn't seem to have any effect.

daurnimator commented 7 years ago

so you'd probably be looking for a replacement quite soon.. I guess taking over mainly involves tagging it when it's stable, and being in charge of the rockspec. So I'd like it if another volunteer could be found, and another may have to be found soon anyway.

There can be more than one :)

If daurnimator could be made a collaborator that would be great

I have been made a collaborator now.

peterbillam commented 7 years ago

There can be more than one :)

Er, perhaps I should be a collaborator too, then ?

Back to quiet=yes : perhaps we could add a function corresponding to https://xiph.org/ao/doc/ao_append_global_option.html so that the programmers could do global_option('quiet', 'yes') or append_global_option('quiet', 'yes') The libao function just has (key, value) arguments, so maybe we should copy that, or maybe we should expect a table, for compatibility with the option table of openLive()... But I haven't tried it, and I'm a bit nervous about the word 'append' I mean will it also do 'overrule' ?

daurnimator commented 7 years ago

Back to quiet=yes : perhaps we could add a function corresponding to https://xiph.org/ao/doc/ao_append_global_option.html so that the programmers could do global_option('quiet', 'yes') or append_global_option('quiet', 'yes') The libao function just has (key, value) arguments, so maybe we should copy that, or maybe we should expect a table, for compatibility with the option table of openLive()... But I haven't tried it, and I'm a bit nervous about the word 'append' I mean will it also do 'overrule' ?

Looks like lao already supports passing options (in table form) as third argument to openLive. e.g. device = ao.openLive(ao.defaultDriverId(), format, {quiet="any string"})

peterbillam commented 7 years ago

Looks like lao already supports passing options (in table form) as third argument to openLive e.g. device = ao.openLive(ao.defaultDriverId(), format, {quiet="any string"})

Yes, but they're not the global options, they're just the driver options, and setting quiet='yes' there has no effect, because the debug messages have already been printed. Like in https://xiph.org/ao/doc/config.html it distinguishes between AO Options and Driver Options. I'm hoping AO Options means the same as global_options, and I'm also hoping that calling ao_append_global_option after requiring the module, but before calling openLive or openFile will set quiet='yes' soon enough to stop those messages... It's a separate issue; we should get a new lao version onto luarocks.org first, meanwhile I'll do some experimenting. We do already have the config files as a mechanism. ( The config files can be used to set both AO Options, and Driver Options )

daurnimator commented 7 years ago

Looks like the initial print comes from the automatic internal ao_initialize call. Could you make a new issues requesting its removal? (perhaps we can e.g. auto-initialize on the first call to a function?)

peterbillam commented 7 years ago

Could you make a new issues requesting its removal? You mean here in https://github.com/TheLinx/lao/ ? I've just registered myself at xiph.org (which was fraught), but now I realise I was probably on the wrong track ...

(perhaps we can e.g. auto-initialize on the first call to a function?) Yes! or if that function is append_global_option then we call that first and ao_initializesecond (I hope it works that way round), whereas if it's some other function then we call ao_initialize first and the function second.

daurnimator commented 7 years ago

Could you make a new issues requesting its removal? You mean here in https://github.com/TheLinx/lao/ ?

Yep :) I already wrote a PR. see #6.