Re: [Discuss-gnuradio] Some misconceptions about the "peak_detector2" block

[Tom Rondeau]

On Tue, Apr 21, 2015 at 2:23 PM, Frank Fu <address@hidden> wrote:

Thanks for your response, Tom.  So basically, the documentation needs to be updated to better reflect the desired behavior of the block.  My interpretation of the behavior would be something like “The look_ahead value creates a minimum search window.  The peak is first searched within this window.  If the last sample in this window is the peak, the block continues searching for the peak until the input flattens or falls down”.

So, I can see this block containing three states: peak not found, peak search in window, and extended peak search.  The current trouble, as I see it, is during the window search phase.  A lot of buggy things happen when the window search isn’t fully completed within a work call.  If set_output_multiple is not desired, how about creating a saved output buffer of length look_ahead?  During the window search phase, the output state could be saved until enough inputs come in. The block would have to use general_work instead, because there would be a temporary point where input_samples are consumed, but no outputs are written.

Frank,

See the discussion in http://gnuradio.org/redmine/issues/783

I've created a pull request to add Achilleas' version to maint to replace the current one. There are still some potential issues that we might run in to, but this is still a much better answer, and my concerns might be completely unfounded. The set_output_multiple issue is something that I've seen, but it's anecdotal evidence. We'd need benchmarking before trying to develop a different solution.

Finally, I hope you would consider creating a third peak detector block that would perform the finite window search.  It would be extremely useful, as it solves many kinds of signal detection problems for a receiver.

Frank

I would consider receiving a pull request for such a block :)

But if anyone works on this, please don't call it peak_detector3! Give it a more meaningful name for what it's doing.

Thanks!

Tom

 

On Apr 21, 2015, at 10:26 AM, Tom Rondeau <address@hidden> wrote:

>
>
> So I pretty much agree with this now. In the current form of the block, the function of the look_ahead doesn't behave the way that you described here, Frank. However, looking at the (sparse) documentation for this block, I can now see why everyone thinks that's how it's supposed to behave. From [1]:
>
>   look_ahead: The look-ahead value is used when the threshold is found to locate the peak within this range.
>
> And if that's how everyone expects it to behave, which is what I'm hearing, then we should correct that.
>
> Here are a couple of points that I'd like to consider for reworking this block. First, the flowgraph must finish no matter how many items are passed to it. The block was initially designed for an OFDM preamble detector, so we can't have it just waiting around for more samples to come in, which will add to latency.
>
> I think that the variable "look_ahead" should probably be renamed to "window" or something. The documentation for this block also needs a lot of work to avoid ambiguity or confusion about what it's really supposed to do and how it works. Better QA code and an example would be really useful, too.
>
> Finally, I don't think that set_output_multiple is the right answer here. I'd rather it save state between calls to work if it's looking in the current window. We shouldn't have a block wait for thousands of samples (the default look_ahead is 1000) before making a decision. And using set_output_multiple with large values has, in my experience, a performance hit on a flowgraph.
>
> Tom
>
> [1] http://gnuradio.org/doc/doxygen/classgr_1_1blocks_1_1peak__detector2__fb.html
>
>