I attach the patch for this correction (for some reason I cannot git push...)
Achilleas On Wed, Oct 9, 2013 at 12:59 PM, Achilleas Anastasopoulos <[email protected] > wrote: > Maybe I am wrong, but here is the idea: > > the original taps are "taps". > then inside the freq_xlating filter new "combined" taps are generated > as follows > > comb_t = taps_t *exp(-j A t) > > then the COMBINED filter is reversed. > The effect of that is that essentially we have the filter > > reversed_t = taps_{-t} *exp( + j A t) > > If we drop the reversing part from the process (commenting out one line of > code) we will end up > with the filter nonreversed with > > nonrevered_t = comb_t = taps_t *exp(-j A t) > > Comparing the reveresed and non-reversed we see that even when taps are > symmetric, the frequency sign gas changed so we need to change it back! > > let me know if i am missing something, > Achilleas > > > > On Wed, Oct 9, 2013 at 11:02 AM, Tom Rondeau <[email protected]> wrote: > >> On Wed, Oct 9, 2013 at 10:45 AM, Achilleas Anastasopoulos >> <[email protected]> wrote: >> > I will submit the patch. >> > >> > regarding the sign change in frequency, I didn't mean to change the >> > convention: >> > the sign change IS REQUIRED in order to KEEP the convention because now >> > the taps are not reversed... >> > >> > Achilleas >> >> Sorry, Achilleas, I'm not seeing it. In the common case of a symmetric >> FIR filter, the reverse function doesn't change any behavior, but the >> minus sine definitely does. >> >> I don't see how reversing the order of the filter taps and changing >> the sign have anything to do with each other. >> >> Tom >> >> >> > On Wed, Oct 9, 2013 at 9:20 AM, Tom Rondeau <[email protected]> wrote: >> >> >> >> On Tue, Oct 8, 2013 at 9:39 PM, Achilleas Anastasopoulos >> >> <[email protected]> wrote: >> >> > >> >> > I was playing around with >> >> > >> >> > fir_filter_XXX >> >> > >> >> > and >> >> > >> >> > freq_xlating_fir_filter_XXX >> >> > >> >> > and noticed that the two do not give the same output >> >> > for the same input (and center_freq=0 in the xlating filter). >> >> > >> >> > Looking at the implementation of the latter >> >> > it is obvious why: the taps are reversed in the line: >> >> > >> >> > std::reverse(ctaps.begin(), ctaps.end()); >> >> > >> >> > For consistency the taps should not be reversed (as in all other >> >> > filters) >> >> > We also need to set >> >> >> >> Yes, please submit a patch for this. The taps are reversed inside the >> >> fir filters, so this is redundant and confusing. Most people probably >> >> use symmetric filter taps, which is why it has not been found. >> >> >> >> > float fwT0 = 2 * M_PI * d_center_freq / d_sampling_freq; >> >> > >> >> > (instead of the minus sign in the code). >> >> > >> >> > unless there is an objection, I will go ahead and push a correction, >> >> > Achilleas >> >> >> >> Don't change the sign of the frequency. I know this is controversial, >> >> but from my experience with users, more people find the current way >> >> easier to understand. We're telling the filter what the center >> >> frequency is, which means that we will take a signal at Fc and >> >> downshift it to DC. To me, if we're on carrier Fc and we specify -Fc >> >> as the "Center Frequency", that's more confusing. >> >> >> >> Tom >> > >> > >> > >
0001-fixed-freq_xlating_filter-block-so-that-it-does-not-.patch
Description: Binary data
_______________________________________________ Discuss-gnuradio mailing list [email protected] https://lists.gnu.org/mailman/listinfo/discuss-gnuradio
