Skip to content

Update ruby_whisper_params.c#3022

Merged
KitaitiMakoto merged 1 commit intoggml-org:masterfrom
Olli:patch-2
Apr 9, 2025
Merged

Update ruby_whisper_params.c#3022
KitaitiMakoto merged 1 commit intoggml-org:masterfrom
Olli:patch-2

Conversation

@Olli
Copy link
Copy Markdown
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

@Olli
Update ruby_whisper_params.c
1ee13e0 
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
Copy Markdown
Contributor

KitaitiMakoto commented Apr 9, 2025

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
Copy Markdown
Contributor Author

Olli commented Apr 9, 2025 •
edited
Loading

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
Copy Markdown
Contributor

KitaitiMakoto commented Apr 9, 2025

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
Copy Markdown
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
@bygreencn
Merge remote-tracking branch 'ggerganov/master'
1167dc4 
* 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
@bygreencn
Merge remote-tracking branch 'ggerganov/master'
16e422c 
* 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

Reviewers

1 more reviewer

@KitaitiMakoto KitaitiMakoto KitaitiMakoto approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Olli @KitaitiMakoto