[shared-module/audioio/WaveFile.h] Change sample_rate from uint16_t t…

[C47D]

…o uint32_t so it matches the sample rate type parsed from the WAV header format, fix #1922

[dhalbert]

I checked other uses of sample_rate in other audio native classes, and they are uint32. The WAV file header itself uses 4 bytes for sample rate. So this looks safe and consistent.

[C47D]

@dhalbert Hi, i was planning to check what is the maximum sample rate allowed for wav files. But as you say it seems its safe to merge.

For reference:

Appears to me it's because sample_rate is an uint16_t in the audioio_wavefile_obj_t type.

circuitpython/shared-module/audioio/WaveFile.h

Lines 34 to 55 in c5a4e19

	typedef struct {
	mp_obj_base_t base;
	uint8_t* buffer;
	uint32_t buffer_length;
	uint8_t* second_buffer;
	uint32_t second_buffer_length;
	uint32_t file_length; // In bytes
	uint16_t data_start; // Where the data values start
	uint8_t bits_per_sample;
	uint16_t buffer_index;
	uint32_t bytes_remaining;
	uint8_t channel_count;
	uint16_t sample_rate;
	uint32_t len;
	pyb_file_obj_t* file;
	uint32_t read_count;
	uint32_t left_read_count;
	uint32_t right_read_count;
	} audioio_wavefile_obj_t;

While the parsed sample_rate of the wave header format is uint32_t:

circuitpython/shared-module/audioio/WaveFile.c

Lines 38 to 46 in c5a4e19

	struct wave_format_chunk {
	uint16_t audio_format;
	uint16_t num_channels;
	uint32_t sample_rate;
	uint32_t byte_rate;
	uint16_t block_align;
	uint16_t bits_per_sample;
	uint16_t extra_params; // Assumed to be zero below.
	};

And i think it gets truncated here:

circuitpython/shared-module/audioio/WaveFile.c

Line 86 in c5a4e19

self->sample_rate = format.sample_rate;

From here.

[tannewt]

Looks good to me! Thank you!

[kevinjwalters]

Thanks for quick fix.

[kevinjwalters]

I was thinking about this a bit more. This can be picked up by the compiler but the warning isn't commonly enabled perhaps because of the traditional habits of the typical C programmer. gcc's -Wall and -Wextra do not enable this warning but -Wconversion does, an example:

$ cat -n integer-assignment-size.c | sed -n '12,15p; 21,22p'
    12    unsigned int a;
    13    uint32_t b;
    14    unsigned short c;
    15    uint16_t d;
    21    c = a;
    22    d = (uint16_t)b;

$ gcc -Wconversion -o integer-assignment-size integer-assignment-size.c
integer-assignment-size.c: In function ‘main’:
integer-assignment-size.c:21:3: warning: conversion to ‘short unsigned int’ from ‘unsigned int’ may alter its value [-Wconversion]
   c = a;
   ^

It might be worth running -Wconversion (cf -Wno-sign-conversion) or its equivalent over the code base to see what the SNR looks like and putting in a task to clean them up with review and fix or explicit conversion (e.g. line 22) to remove warning? Even the explicit cast comes with its own risks, of course.