Re: kern/56644: Kernel assertion in audio.c

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

The following reply was made to PR kern/56644; it has been noted by GNATS.

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: isaki%NetBSD.org@localhost
Cc: gnats-bugs%NetBSD.org@localhost
Subject: Re: kern/56644: Kernel assertion in audio.c
Date: Sat, 29 Jan 2022 00:55:18 +0000

 The logic in audio_rmixer_process right above this assertion seems
 wrong:
 
 		/* If the track buffer is full, discard the oldest one? */
 		input = track->input;
 		if (input->capacity - input->used < mixer->frames_per_block) {
 			int drops = mixer->frames_per_block -
 			    (input->capacity - input->used);
 			track->dropframes += drops;
 			TRACET(4, track, "drop %d frames: inp=%d/%d/%d",
 			    drops,
 			    input->head, input->used, input->capacity);
 			auring_take(input, drops);
 		}
 		KASSERTMSG(input->used % mixer->frames_per_block == 0,
 		    "input->used=%d mixer->frames_per_block=%d",
 		    input->used, mixer->frames_per_block);
 
 If the assertion holds before the branch, and the branch is taken,
 then the assertion is guaranteed to fail after the branch.
 
 Shouldn't it discard exactly one block's worth of frames instead?  Any
 smaller amount will make the assertion fail.  Or am I confused, and is
 the assertion not expected to hold before the branch?
 
 diff --git a/sys/dev/audio/audio.c b/sys/dev/audio/audio.c
 index 176749ac449f..519cc098d03a 100644
 --- a/sys/dev/audio/audio.c
 +++ b/sys/dev/audio/audio.c
 @@ -5879,8 +5879,7 @@ audio_rmixer_process(struct audio_softc *sc)
  		/* If the track buffer is full, discard the oldest one? */
  		input = track->input;
  		if (input->capacity - input->used < mixer->frames_per_block) {
 -			int drops = mixer->frames_per_block -
 -			    (input->capacity - input->used);
 +			int drops = mixer->frames_per_block;
  			track->dropframes += drops;
  			TRACET(4, track, "drop %d frames: inp=%d/%d/%d",
  			    drops,