BatchDrake / sigutils

Small signal processing utility library
https://batchdrake.github.io/sigutils
GNU General Public License v3.0
71 stars 29 forks source link

uninitialized var warning #21

Closed jeffpc closed 3 years ago

jeffpc commented 3 years ago

Recent TV related changes introduced a warning:

/home/jeffpc/bin/dsp/include/sigutils/sigutils/clock.h:70:20: warning: variable 'phase' is uninitialized
      when used here [-Wuninitialized]
      if (SU_FLOOR(phase) == 0) {
                   ^~~~~
/home/jeffpc/bin/dsp/include/sigutils/sigutils/clock.h:61:23: note: initialize the variable 'phase' to
      silence this warning
  SUFLOAT alpha, phase;
                      ^
                       = 0.0
1 warning generated.

I'm not sure what it should actually say. I'm guessing that phase is supposed to be initialized to self->phase at some point in that function.

BatchDrake commented 3 years ago

The funny story here is that I've been suffering aliasing with fractional baudrates for a while and I spent hours staring at this function trying to understand what I did wrong. I guess it was so obvious that I couldn't see it until I got this warning.

Anyways, it should be fixed now. Thanks!

jeffpc commented 3 years ago

On Tue, Aug 11, 2020 at 11:22:01 -0700, Gonzalo José Carracedo Carballal wrote:

The funny story here is that I've been suffering aliasing with fractional baudrates for a while and I spent hours staring at this function trying to understand what I did wrong. I guess it was so obvious that I couldn't see it until I got this warning.

Anyways, it should be fixed now. Thanks!

The change fixes that warning. :)

It turns out there is another (much less severe) warning that I didn't notice before:

/home/jeffpc/src/oss/dsp/sigutils/sigutils/tvproc.c:700:27: warning: absolute value function 'fabsf' given
      an argument of type 'double' but has parameter of type 'float' which may cause truncation of value
      [-Wabsolute-value]
  err_vsync_offset  = 2 * SU_ABS(.5 - vsync_pos / self->est_line_len);
                          ^

(Same thing on lines 701, 767, and 768 as well)

Basically, SU_ABS is defined to be fabsf (which operates on floats) but tvproc.c uses it to work on doubles.

BatchDrake commented 3 years ago

That is probably because the .5, which should be .5f. I'll fix it in a while.

El mar., 11 ago. 2020 21:12, jeffpc notifications@github.com escribió:

On Tue, Aug 11, 2020 at 11:22:01 -0700, Gonzalo José Carracedo Carballal wrote:

The funny story here is that I've been suffering aliasing with fractional baudrates for a while and I spent hours staring at this function trying to understand what I did wrong. I guess it was so obvious that I couldn't see it until I got this warning.

Anyways, it should be fixed now. Thanks!

The change fixes that warning. :)

It turns out there is another (much less severe) warning that I didn't notice before:

/home/jeffpc/src/oss/dsp/sigutils/sigutils/tvproc.c:700:27: warning:
absolute value function 'fabsf' given
an argument of type 'double' but has parameter of type 'float' which may
cause truncation of value
[-Wabsolute-value]
err_vsync_offset = 2 * SU_ABS(.5 - vsync_pos / self->est_line_len);
^

(Same thing on lines 701, 767, and 768 as well)

Basically, SU_ABS is defined to be fabsf (which operates on floats) but tvproc.c uses it to work on doubles.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/BatchDrake/sigutils/issues/21#issuecomment-672206054, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEVET7KFH5MQM7Q3QY7GMLSAGJ3DANCNFSM4P3KAUVA .

BatchDrake commented 3 years ago

It should be fixed now. I also rewrote some comparisons that were a bit difficult to read.

jeffpc commented 3 years ago

Builds fine, and a 10 second test replay of a raw file didn't reveal any issue :)