Skip to content

Update ruby_whisper_params.c #3022

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

Merged
merged 1 commit into from
Apr 9, 2025
Merged

Update ruby_whisper_params.c #3022

merged 1 commit into from
Apr 9, 2025

Conversation

Olli
Copy link
Contributor

@Olli Olli commented Apr 9, 2025

Change param_names and values not to be references for rb_get_kwargs - so it can be compiled on ruby 3.3.6 and 3.4.1

Change param_names and values not to be references for rb_get_kwargs - so it can be compiled on ruby 3.3.6 and 3.4.1
@KitaitiMakoto
Copy link
Collaborator

Thanks for the pull request.

Can you show your environment and error logs? I tried Ruby 3.1 and 3.2 on my machine (macOS Apple Silicon) and Ubuntu Docker image but they compiled successfully.

And according to API reference, rb_get_kwargs function's second and fifth arguments (here, param_names and values respectively) should be references.

@Olli
Copy link
Contributor Author

Olli commented Apr 9, 2025

My setup is rvm with ruby 3.3.6 or 3.4.1
besides a lot of warnings here are the lines where the compiler stops:

ruby_whisper_params.c:921:26: error: passing argument 2 of ‘rb_get_kwargs’ from incompatible pointer type [-Wincompatible-pointer-types]
  921 |   rb_get_kwargs(kw_hash, &param_names, 0, RUBY_WHISPER_PARAMS_PARAM_NAMES_COUNT, &values);
      |                          ^~~~~~~~~~~~
      |                          |
      |                          ID (*)[30] {aka long unsigned int (*)[30]}
In file included from /home/xxx/.rvm/rubies/ruby-3.4.1/include/ruby-3.4.0/ruby/ruby.h:33:
/home/xxx/.rvm/rubies/ruby-3.4.1/include/ruby-3.4.0/ruby/internal/eval.h:380:49: note: expected ‘const ID *’ {aka ‘const long unsigned int *’} but argument is of type ‘ID (*)[30]’ {aka ‘long unsigned int (*)[30]’}
  380 | int rb_get_kwargs(VALUE keyword_hash, const ID *table, int required, int optional, VALUE *values);
      |                                       ~~~~~~~~~~^~~~~
ruby_whisper_params.c:921:82: error: passing argument 5 of ‘rb_get_kwargs’ from incompatible pointer type [-Wincompatible-pointer-types]
  921 |   rb_get_kwargs(kw_hash, &param_names, 0, RUBY_WHISPER_PARAMS_PARAM_NAMES_COUNT, &values);
      |                                                                                  ^~~~~~~
      |                                                                                  |
      |                                                                                  VALUE (*)[30] {aka long unsigned int (*)[30]}
/home/xxx/.rvm/rubies/ruby-3.4.1/include/ruby-3.4.0/ruby/internal/eval.h:380:91: note: expected ‘VALUE *’ {aka ‘long unsigned int *’} but argument is of type ‘VALUE (*)[30]’ {aka ‘long unsigned int (*)[30]’}
  380 | int rb_get_kwargs(VALUE keyword_hash, const ID *table, int required, int optional, VALUE *values);

If I change it like in the pull request than the compilation process works fine and whisper seem to also work as it supposed to.
I've also looked up the documentation. Because I didn't know why it works I've looked up in an LLM and ask why this works and it seems that it works because if you give an array to the function its automatically a reference to the first element of the array. Not sure if it's correct.

@KitaitiMakoto
Copy link
Collaborator

Ah, param_names and values are declared as arrays and it means they are pointers in C. You're right. My compiler (Homebrew clang version 20.1.2) is too smart to compile my wrong code.

Thank you!

@KitaitiMakoto KitaitiMakoto merged commit ef6cf35 into ggml-org:master Apr 9, 2025
51 checks passed
@Olli
Copy link
Contributor Author

Olli commented Apr 9, 2025

thx for explanation :-) I'm getting smarter every day

bygreencn added a commit to bygreencn/whisper.cpp that referenced this pull request Jun 29, 2025
* ggerganov/master: (25 commits)
  examples : add HEAPU8 to exported runtime methods (ggml-org#3062)
  ruby : make Ruby bindings installed with build options (ggml-org#3056)
  whisper : add no_context parameter to whisper_params (ggml-org#3045)
  examples : add FFmpeg v7.0 support to ffmpeg-transcode.cpp (ggml-org#3038)
  ruby: use CMake in build process (ggml-org#3043)
  docs : update README.md to note newer nvidia gpus (ggml-org#3031)
  addon.node : support max_context api for addon.node (ggml-org#3025)
  whisper : reduce delta_min from 1000ms to 100ms (ggml-org#3028)
  docs : document how to use 'WHISPER_FFMPEG' build option (ggml-org#3029)
  docs : fix README.md (ggml-org#3024)
  xcf : use check for visionos build version (ggml-org#3021)
  ruby : fix types of arguments for rb_get_kwargs in ruby_whisper_params.c (ggml-org#3022)
  ruby : Update uri.rb (ggml-org#3016)
  models : fix dead link to models in readme (ggml-org#3006)
  ruby : change homepage URI in Ruby gemspec (ggml-org#3007)
  tests : add script to benchmark whisper.cpp on LibriSpeech corpus (ggml-org#2999)
  whisper : fix "bench-all outputs an invalid result on larger models" (ggml-org#3002)
  rename : ggerganov -> ggml-org (ggml-org#3005)
  examples : update server.py to match github pages app [no ci] (ggml-org#3004)
  whisper.wasm : fix unknown language issue (ggml-org#3000)
  ...
bygreencn added a commit to bygreencn/whisper.cpp that referenced this pull request Jun 29, 2025
* ggerganov/master: (25 commits)
  examples : add HEAPU8 to exported runtime methods (ggml-org#3062)
  ruby : make Ruby bindings installed with build options (ggml-org#3056)
  whisper : add no_context parameter to whisper_params (ggml-org#3045)
  examples : add FFmpeg v7.0 support to ffmpeg-transcode.cpp (ggml-org#3038)
  ruby: use CMake in build process (ggml-org#3043)
  docs : update README.md to note newer nvidia gpus (ggml-org#3031)
  addon.node : support max_context api for addon.node (ggml-org#3025)
  whisper : reduce delta_min from 1000ms to 100ms (ggml-org#3028)
  docs : document how to use 'WHISPER_FFMPEG' build option (ggml-org#3029)
  docs : fix README.md (ggml-org#3024)
  xcf : use check for visionos build version (ggml-org#3021)
  ruby : fix types of arguments for rb_get_kwargs in ruby_whisper_params.c (ggml-org#3022)
  ruby : Update uri.rb (ggml-org#3016)
  models : fix dead link to models in readme (ggml-org#3006)
  ruby : change homepage URI in Ruby gemspec (ggml-org#3007)
  tests : add script to benchmark whisper.cpp on LibriSpeech corpus (ggml-org#2999)
  whisper : fix "bench-all outputs an invalid result on larger models" (ggml-org#3002)
  rename : ggerganov -> ggml-org (ggml-org#3005)
  examples : update server.py to match github pages app [no ci] (ggml-org#3004)
  whisper.wasm : fix unknown language issue (ggml-org#3000)
  ...
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

Successfully merging this pull request may close these issues.

2 participants