Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AudioBufferSourceNode: fix panic when duration exceeds buffer len #452

Merged
merged 2 commits into from
Feb 18, 2024

Conversation

orottier
Copy link
Owner

This fixes the last remaining panic occuring in the wpt suite:

  wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-start.html

  √ # AUDIT TASK RUNNER STARTED.
  √ > [Tests AudioBufferSourceNode start()]
Panic occurred in Audio Processor: 'index out of bounds: the len is 8 but the index is 8'. Removing node from graph.
  √   Case 0: start(when): implicitly play whole buffer from beginning to end is identical to the array [0,1,2,3,4,5,6,7,0,0,0,0,0,0,0,0...].
  √   Case 0: start(when): implicitly play whole buffer from beginning to end: tail contains only the constant 0.
  √   Case 1: start(when, 0): play whole buffer from beginning to end explicitly giving offset of 0 is identical to the array [0,1,2,3,4,5,6,7,0,0,0,0,0,0,0,0...].
  √   Case 1: start(when, 0): play whole buffer from beginning to end explicitly giving offset of 0: tail contains only the constant 0.
  √   Case 2: start(when, 0, 8_frames): play whole buffer from beginning to end explicitly giving offset of 0 and duration of 8 frames is identical to the array [0,1,2,3,4,5,6,7,0,0,0,0,0,0,0,0...].
  √   Case 2: start(when, 0, 8_frames): play whole buffer from beginning to end explicitly giving offset of 0 and duration of 8 frames: tail contains only the constant 0.
  √   Case 3: start(when, 4_frames): play with explicit non-zero offset is identical to the array [4,5,6,7,0,0,0,0,0,0,0,0,0,0,0,0...].
  √   Case 3: start(when, 4_frames): play with explicit non-zero offset: tail contains only the constant 0.
  √   Case 4: start(when, 4_frames, 4_frames): play with explicit non-zero offset and duration is identical to the array [4,5,6,7,0,0,0,0,0,0,0,0,0,0,0,0...].
  √   Case 4: start(when, 4_frames, 4_frames): play with explicit non-zero offset and duration: tail contains only the constant 0.
  √   Case 5: start(when, 7_frames): play with explicit non-zero offset near end of buffer is identical to the array [7,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0...].
  √   Case 5: start(when, 7_frames): play with explicit non-zero offset near end of buffer: tail contains only the constant 0.
  √   Case 6: start(when, 8_frames): play with explicit offset at end of buffer is identical to the array [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0...].
  √   Case 6: start(when, 8_frames): play with explicit offset at end of buffer: tail contains only the constant 0.
  √   Case 7: start(when, 9_frames): play with explicit offset past end of buffer is identical to the array [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0...].
  √   Case 7: start(when, 9_frames): play with explicit offset past end of buffer: tail contains only the constant 0.
  × X Case 8: start(when, 0, 15_frames): play with whole buffer, with long duration (clipped) expected to be equal to the array [0,1,2,3,4,5,6,7,0,0,0,0,0,0,0,0...] but differs in 7 places:
  	Index	Actual			Expected
  	[1]	0.0000000000000000e+0	1.0000000000000000e+0
  	[2]	0.0000000000000000e+0	2.0000000000000000e+0
  	[3]	0.0000000000000000e+0	3.0000000000000000e+0
  	[4]	0.0000000000000000e+0	4.0000000000000000e+0
  	...and 3 more errors.

It took me a while to hunt this one down and somehow I did not succeed to write a regression test :( I just could not reproduce the bug and did not feel to port the full wpt test to rust..

@orottier orottier requested a review from b-ma February 15, 2024 07:52
@b-ma
Copy link
Collaborator

b-ma commented Feb 18, 2024

Hey, nice catch!

I managed to create a failing test for that:

    #[test]
    // regression test for #452
    // - fast track
    // - duration not set so `self.duration` is `f64::MAX`
    // - stop time is > buffer length
    fn test_end_of_file_fast_track_2() {
        let sample_rate = 48_000.;
        let mut context = OfflineAudioContext::new(1, RENDER_QUANTUM_SIZE, sample_rate);

        let mut buffer = context.create_buffer(1, 5, sample_rate);
        let data = vec![1.; 1];
        buffer.copy_to_channel(&data, 0);

        let mut src = context.create_buffer_source();
        src.connect(&context.destination());
        src.set_buffer(buffer);
        // play in fast track
        src.start_at(0.);
        // stop after end of buffer but before the end of render quantum
        src.stop_at(125. / sample_rate as f64);

        let result = context.start_rendering_sync();
        let channel = result.get_channel_data(0);

        let mut expected = vec![0.; 128];
        expected[0] = 1.;

        assert_float_eq!(channel[..], expected[..], abs_all <= 0.);
    }

@orottier orottier merged commit 48caf24 into main Feb 18, 2024
3 checks passed
@orottier orottier deleted the bugfix/buffersource-panic-duration branch February 18, 2024 09:25
@orottier
Copy link
Owner Author

Thanks, that does the trick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants