Don't lazy initialize PS decoder about pocketsphinx-ruby

[nshmyrev]

Looks cool, thank you!

[ojak]

I'm still seeing that Decoder.new is still overwriting explicit configuration flags, and it looks as though cmu-pocketsphinx/bin/pocketsphinx_batch might be automatically reading in the library's feat.params file and causing the problems. Here's what I'm seeing after the above-listed patch:

> configuration = Pocketsphinx::Configuration.default
> configuration['cmninit']
=> "8.0"
> decoder = Pocketsphinx::Decoder.new(configuration)
> decoder.configuration['cmninit']
=> "40,3,-1"

Since the example above is passing an explicit configuration object to the Decoder initializer, I would expect that the Decoder would obey those parameters and initialize with the configuration parameters that are being passed in (ie. decoder.configuration['cmninit'] should still be equal to 8 after initialization).

I'm thinking that pocketsphinx-ruby should read the feat.params file and handle all of the configuration of the tool, instead of deferring configuration to the cmu-pocketsphinx library. So, Decoder.new() and Decoder.new(Configuration.default) should really result in identical decoders. I took a look at the cmu-pocketsphinx libraries, but I couldn't quite tell without a deeper dive how feat.params is treated by the library.

@watsonbox @nshmyrev Do you know of any way to have cmu-pocketsphinx/bin/pocketsphinx_batch ignore feat.params altogether so we can just pass all the configuration variables in directly without them being hijacked and overwritten by the underlying library. I think we're looking for behavior similar to #reinit_decoder?

[watsonbox]

@ojak Yes as I said in #10: "Note that there would still be a 'two-phase' initialization - one when creating a Configuration and one when creating a Decoder." In Pocketsphinx terms, the Configuration represents the command line options (or user options) which are passed to the decoder. Then ps_init initializes a decoder which is represented by Decoder.

This gem is intended to wrap Pocketsphinx and while I'm not against adding a layer of useful behavior where absolutely necessary (i.e. #10), I don't want it to become a replacement implementation for various parts of Pocketsphinx. In this case, digging around doing custom initialization with the feat.params feels like precisely that (i.e. providing a duplicate implementation of the private ps_init_defaults).

It looks like in theory this could be resolved quite easily in Pocketsphinx, by adding (or moving) the call to ps_init_defaults to the end of ps_default_search_args. Or even by exposing ps_init_defaults in the public API, with the configuration as a parameter instead of the decoder. That's something that would need to be raised in the Pocketsphinx forums.

I'll add a note to the Decoder section of the README to avoid confusion as much as possible for now.

[ojak]

@watsonbox Ok. If that's the case, passing a configuration directly into Decoder.new will never work. Why not just remove the configuration parameter from the initializer entirely and replace it with Pocketsphinx::Configuration.default?

That would at least help further reduce confusion, since the Decoder.new really doesn't obey the configuration parameter since it's always going to be overwritten anyhow?

[watsonbox]

Not sure what you mean when you say passing a configuration will never work. There are different configurations described in the documentation for keyword spotting and using grammars. These are just two possibilities that I've singled out for special attention but there could be any number of reasons why a user might wish to adjust the decoder configuration.

The decoder only overwrites a small subset of configuration settings relating to the acoustic model.

[ojak]

Sorry. To clarify, what I mean is that the small subset of overwritten parameters you're referring to are absolutely critical to providing an accurate decode, and without them, the decoder fails to accurately decode.

I agree with you with regards to not wanting to duplicate the configuration of the underlying Pocketsphinx library's own local configuration, which is totally fine. But the setup here results in a silent replacement of wrapper configuration variables that most developer will reasonably expect are reliably configuring the tool. Since the configuration problem/bug is effectively being hidden during the wrapper communication with the library, this will pretty much guarantee confusion. I'm just suggesting some sort of fix based on my own lengthy experience of trying to get the gem to work (and nearly aborting using the gem at all).

It's an amazing and powerful gem, and I'm really just trying to help figure out a way to make it more developer-friendly and accessible.

[watsonbox]

Okay no worries - I propose to close this issue off and either continue discussion on your PR, or on a new issue/PR related to any other concrete proposals for improvement.

This issue was really just to ensure that all config init was complete before starting the first decode so as to align the gem's behavior with that of Pocketsphinx itself.

The 'silent' replacement of wrapper variables based on the acoustic model is a Pocketsphinx feature, analogous to the same replacement of command line arguments when using it on the command line. For now at least, that's just how it works.

[ojak]

Ok. Is there a running TODO list for this gem? This would probably be a good enhancement to add to that list.

[watsonbox]

I've been using issues and PRs for that, as long as the proposal is relatively concrete. That way it's easier to discuss, and things don't simply sit on the list forever.