Add Timer, add audio queuing, and make Window return current dimensions

[mattrberry]

• add sdl timer (+ added sample)
• add audio queueing (+ added sample)
• Make Window#width and Window#height return current dimensions

This will allow me to drop the additional bindings I've maintained here for awhile

[ysbaddaden]

I'm wary of SDL timers: how are they implemented in SDL?

• Do they run in a thread handled by SDL that Crystal doesn't know about about (expect segfaults)?
• Do they block the current thread, which means that they will block Crystal's event loop, or the GC from running (expect hangups)?

We should probably have an implementation in Crystal. For example SDL_delay(50) must be replaced by sleep(50.milliseconds). It relies on libevent (and some fiber context switches), so it's likely less precise than SDL timers, but it won't block the whole program, only the current Fiber that wants to wait: the rest of the program can keep running.

I'm less wary of audio. I assume SDL plays music in a thread entirely controlled by SDL, without any callbacks to Crystal?

[mattrberry]

I don't know the answer to your questions regarding SDL timers, which is why I've just added the bindings and didn't touch the crystal wrapper around them. I feel like the bindings should exist regardless of how the wrapper is implemented

[ysbaddaden]

SDL timer callbacks are executed in a thread created by SDL that Crystal knows nothing about. Running the sample with -Dpreview_mt will likely cause segfaults. See https://github.com/libsdl-org/SDL/blob/main/src/timer/SDL_timer.c#L168

SDL_Delay is merely a convenience on top of libc's nanosleep function for POSIX (at least) which will block the current thread, because it's not aware of event loop, and prevent concurrent fibers from running, thus hanging the program for the duration of the delay. We must use Crystal's sleep instead. See https://github.com/libsdl-org/SDL/blob/120c76c84bbce4c1bfed4e9eb74e10678bd83120/src/timer/unix/SDL_systimer.c

So my gut feeling was right: let's not provide bindings for SDL_timer. We can have better abstractions in plain Crystal that will take advantage of Fibers and MT.

[mattrberry]

I still disagree that this means we should omit the bindings. In my opinion, the bindings should be provided even if their usage is discouraged and they're not provided through the API wrapping those bindings. Most people likely shouldn't venture into the LibSDL bindings anyway.

In my use case there's only one fiber, so it's entirely reasonable for the current thread to be blocked by the call to SDL_Delay. Even if the API doesn't wrap that, I'd still like to be able to call LibSDL.delay directly without providing my own bindings

[ysbaddaden]

You can reopen and add your own LibSDL binding in your programs. I won't merge them into the official bindings: they're incompatible with Crystal and even dangerous: they either block/hang the process or segfault your program!

Why bother with SDL_delay() when you can use the safe #sleep? The SDL implementation doesn't even bring any realtime, since it relies on the system's scheduler (so 50ms will likely be a few ms more, same as libevent).

[mattrberry]

Out of curiosity, what makes you call these "the official bindings," or is it simply because you used to be a core team member? Calling this "official" implies it's supported directly by the Crystal team or Manas, and it would also imply a much higher level of polish in my opinion.

[ysbaddaden]

I just meant this shard. Since it binds libsdl, I expect it to be working perfectly with Crystal, and be safe to use.

[ysbaddaden]

Thank you for dropping the SDL_timer bindings! There is just one last reference to LibSDL.delay in the audio sample.

BTW, I didn't notice earlier but there should be an SDL::Audio abstraction on top of the C structs, functions, checking for errors (raise proper SDL::Error exception with an explanatory message), and so on. For example:

class SDL::Audio
  DEFAULT_FREQUENCY = 44100
  DEFAULT_FORMAT = LibSDL::AUDIO_F32SYS

  def self.open(channels, buffer_size, *, frequency = DEFAULT_FREQUENCY, format = DEFAULT_FORMAT)
    audio = new(channels, buffer_size, frequency: frequency, format: format).open
    yield audio
  ensure
    audio.close
  end

  def initialize(channels : Int32, buffer_size : Int32, frequency : Int32 = DEFAULT_FREQUENCY, format : Int32 = DEFAULT_FORMAT)
    @requested_spec = LibSDL::AudioSpec.new
    @requested_spec.freq = frequency
    @requested_spec.format = format
    @requested_spec.channels = channel
    @requested_spec.samples = buffer_size
    @requested_spec.callback = nil
    @requested_spec.userdata = nil

    @obtained_spec = uninitialized LibSDL::AudioSpec
    @closed = true
  end

  def open
    LibSDL.audio_open(pointerof(@requested_spec), pointerof(@obtained_spec))
    raise Error.new("SDL_audio_open") unless ret == 0
    @closed = false
    self
  end

  def close : Nil
    ret = LibSDL.close_audio(...)
    raise Error.new("SDL_close_audio") unless rest == 0
    @closed = true
  end

  def closed?
    @closed
  end

  def play : Nil
    raise AudioClosedError.new if closed?
    # ...
  end

  def pause : Nil
    raise AudioClosedError.new if closed?
    # ...
  end

  def queue(buffer : Slice) : Nil
    raise AudioClosedError.new if closed?
    # ...
  end

  def queue_size : Int32
    raise AudioClosedError.new if closed?
    # ...
  end
end

Yes, writing bindings to an external library is a lot of work (despite Crystal making it simple to call C) 😭

[ysbaddaden]

If you are not interesting in writing safe and C details free bindings, then I can merge just the the SDL_audio bindings.