On Wed, Oct 9, 2013 at 10:45 AM, Achilleas Anastasopoulos <anas...@umich.edu> 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 <t...@trondeau.com> wrote: >> >> On Tue, Oct 8, 2013 at 9:39 PM, Achilleas Anastasopoulos >> <anas...@umich.edu> 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 > > _______________________________________________ Discuss-gnuradio mailing list Discuss-gnuradio@gnu.org https://lists.gnu.org/mailman/listinfo/discuss-gnuradio