1348344 - Add some code to debug the drift on OSX when aggregate device is not enable

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, enhancement, P1)

Description

[Paul Adenot (:padenot)]

I think we don't want most of this in upstream cubeb, so I'm putting it in a patch, like we do for uplift. I can also put it upstream. This is going to be temporary, the long-term plan is to use an aggregate device, but we want to know why it does not work without.

Comment 1

[Paul Adenot (:padenot)]

Attachment: Bug 1348344 - Backed out changeset c72cf98f1527.

Review commit: https://reviewboard.mozilla.org/r/121478/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/121478/

Comment 2

[Paul Adenot (:padenot)]

Setting `CUBEB_AUDIOUNIT_DISABLE_AGGREGATE_DEVICE` in the environment falls back to the old mechanism for duplex, for testing.

We think the buffers in the resampler are growing sometimes, but it's hard to repro, so we want to log their size (asynchronously) while we do calls.

If we feel that we want this upstream, I can prepare another patch.

Comment 3

[Matthew Gregan [:kinetik]]

Comment on attachment 8848577 [details]
Bug 1348344 - Backed out changeset c72cf98f1527.

https://reviewboard.mozilla.org/r/121478/#review123554

Comment 4

[Alex Chronopoulos [:achronop]]

Comment on attachment 8848577 [details]
Bug 1348344 - Backed out changeset c72cf98f1527.

https://reviewboard.mozilla.org/r/121478/#review123914

::: media/libcubeb/src/cubeb_resampler.cpp:80
(Diff revision 1)
>      internal_input_buffer.pop(nullptr, frames_to_samples(output_frames));
>      *input_frames_count = output_frames;
>    }
>  
> +  ALOGV("passthrough: after callback, internal input buffer length: %zu",
> +        internal_input_buffer.length());

Nit: you could mention that this is the number of samples

::: media/libcubeb/src/cubeb_resampler.cpp:80
(Diff revision 1)
>      internal_input_buffer.pop(nullptr, frames_to_samples(output_frames));
>      *input_frames_count = output_frames;
>    }
>  
> +  ALOGV("passthrough: after callback, internal input buffer length: %zu",
> +        internal_input_buffer.length());

Please double check the format specifier. I am not sure if it is the same for every platform.

https://stackoverflow.com/questions/15610053/correct-printf-format-specifier-for-size-t-zu-or-iu

::: media/libcubeb/src/cubeb_resampler.cpp:251
(Diff revision 1)
> +  output_processor->internal_buffer_sizes(output_processor_buffer_sizes);
> +  ALOGV("duplex resampler: after callback, resampling buffer state:"
> +        "input_processor(input: %zu, output: %zu) "
> +        "output_processor(input: %zu, output: %zu) ",
> +        input_processor_buffer_sizes[0], input_processor_buffer_sizes[1],
> +        output_processor_buffer_sizes[0], output_processor_buffer_sizes[1]);

Same as above.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c72cf98f1527
Add a way to disable the aggregate audio device on OSX, and log the resampler internal state. r=achronop,kinetik

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c72cf98f1527

Comment 7

[Paul Adenot (:padenot)]

Comment on attachment 8848577 [details]
Bug 1348344 - Backed out changeset c72cf98f1527.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/121478/diff/1-2/

Comment 8

[Alex Chronopoulos [:achronop]]

Comment on attachment 8848577 [details]
Bug 1348344 - Backed out changeset c72cf98f1527.

https://reviewboard.mozilla.org/r/121478/#review130928

Looks good

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/16b3e165dd90
Backed out changeset c72cf98f1527. r=achronop

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/16b3e165dd90