rem/aufile: test and fix aufile_set_position nread #1010
Conversation
Also fix a bug: wav files can contain data after the data chunk. If we don't update nread in aufile_set_position, we can read invalid data.
|
Tomorrow is release day and this is a bugfix with a unit test. |
test/aupos.c
Outdated
|
|
||
| int err = aufile_open(&af, &prm, path, AUFILE_READ); | ||
| if (err) | ||
| TEST_ERR(err); |
There was a problem hiding this comment.
TEST_ERR already includes the check if (err)
| @@ -307,8 +307,12 @@ int aufile_set_position(struct aufile *af, const struct aufile_prm *prm, | |||
| off_t pos = (off_t)(prm->srate * aufmt_sample_size(prm->fmt) | |||
| * prm->channels * pos_ms / 1000); | |||
|
|
|||
| pos = min((off_t)datasize, pos); | |||
There was a problem hiding this comment.
SEEK_CUR sets the position relative to the current position.
What is datasize after wav_header_decode() returns?
There was a problem hiding this comment.
The size of the data chunk, i.e. the size of the PCM data in bytes.
There was a problem hiding this comment.
The datasize was 1082 instead of 1072 - the file was 67.625 ms long. I've cut it to 67ms exactly now
There was a problem hiding this comment.
I don't understand why the windows tests are failing, I'll try to reproduce
There was a problem hiding this comment.
The
datasizewas 1082 instead of 1072 - the file was 67.625 ms long. I've cut it to 67ms exactly now
So, now you fixed the WAV file? Shouldn't we test that it doesn't read too far?
There was a problem hiding this comment.
Yes, that seemed simpler than fixing the test (i.e. allowing that less than 1ms can be read).
I can extend the test to make sure we don't read too far, that would make sense too.
|
sreimers
changed the title
sreimers
changed the title
Also fix a bug: wav files can contain data after the data chunk. If we don't update
nreadinaufile_set_position, we can read invalid data.(Write a test, find a bug)