Removing Need-Triage-M59 label as per comment #3

Thanks,

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3fe2409358437193a07496c2d0a0b559ef399760

commit 3fe2409358437193a07496c2d0a0b559ef399760
Author: andrew.macpherson <andrew.macpherson@soundtrap.com>
Date: Wed Jul 05 09:49:47 2017

Make OS audio buffer size limits visible.

Make platform-specific audio buffer size limits visible in limits.h and update AudioLatency::GetExactBufferSize() to allow requesting buffer sizes down to the minimum. Update OSX and CRAS to allow audio buffer sizes below their previous minimums when using GetExactBufferSize(), for use in Web Audio AudioContext creation with a latencyHint.

BUG= 708917 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2908073002
Cr-Commit-Position: refs/heads/master@{#484231}

[modify] https://crrev.com/3fe2409358437193a07496c2d0a0b559ef399760/content/renderer/media/renderer_webaudiodevice_impl.cc
[modify] https://crrev.com/3fe2409358437193a07496c2d0a0b559ef399760/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
[modify] https://crrev.com/3fe2409358437193a07496c2d0a0b559ef399760/media/BUILD.gn
[modify] https://crrev.com/3fe2409358437193a07496c2d0a0b559ef399760/media/audio/audio_manager_unittest.cc
[modify] https://crrev.com/3fe2409358437193a07496c2d0a0b559ef399760/media/audio/cras/audio_manager_cras.cc
[modify] https://crrev.com/3fe2409358437193a07496c2d0a0b559ef399760/media/audio/mac/audio_manager_mac.cc
[modify] https://crrev.com/3fe2409358437193a07496c2d0a0b559ef399760/media/audio/pulse/audio_manager_pulse.cc
[modify] https://crrev.com/3fe2409358437193a07496c2d0a0b559ef399760/media/base/audio_latency.cc
[modify] https://crrev.com/3fe2409358437193a07496c2d0a0b559ef399760/media/base/limits.h
[add] https://crrev.com/3fe2409358437193a07496c2d0a0b559ef399760/media/base/mac/audio_latency_mac.cc
[add] https://crrev.com/3fe2409358437193a07496c2d0a0b559ef399760/media/base/mac/audio_latency_mac.h
[modify] https://crrev.com/3fe2409358437193a07496c2d0a0b559ef399760/third_party/WebKit/Source/platform/audio/AudioDestination.cpp

The new AudioManagerTest.CheckMinimumAudioBufferSizesCallbacks test is hitting this assertion failure:
[ RUN      ] AudioManagerTest.CheckMinimumAudioBufferSizesCallbacks
[7918:7974:0705/042850.716929:2587745983:FATAL:pulse_output.cc(136)] Check failed: bytes_to_fill == static_cast<size_t>(params_.GetBytesPerBuffer()) (65472 vs. 65536)
#0 0x0000005ca567 base::debug::StackTrace::StackTrace()
#1 0x0000005d50c1 logging::LogMessage::~LogMessage()
#2 0x00000056975e media::PulseAudioOutputStream::FulfillWriteRequest()

on all of the Linux machines on the chromium.gpu.fyi waterfall:
https://build.chromium.org/p/chromium.gpu.fyi/console


These are physical machines, and the audio_unittests specifically run on them, so this indicates a real problem in the code.

This is the waterfall:

and a few failing builds:
https://build.chromium.org/p/chromium.gpu.fyi/builders/Linux%20Release%20%28NVIDIA%29/builds/49423
https://build.chromium.org/p/chromium.gpu.fyi/builders/Linux%20Debug%20%28NVIDIA%29/builds/37991
https://build.chromium.org/p/chromium.gpu.fyi/builders/Linux%20Debug%20%28Intel%20HD%20530%29/builds/2603

There's still a stale comment about audio_unittests in src/content/test/gpu/generate_buildbot_json.py -- this is why these tests weren't run as part of linux_chromium_rel_ng. Perhaps they should be. Happy to help enable this.

Reverting the patch above to get the bots green again; please tell me if I can do anything to help you diagnose why the test is failing.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0214722083d86f16e50a01d6908c8b3c2f1b8c78

commit 0214722083d86f16e50a01d6908c8b3c2f1b8c78
Author: kbr <kbr@chromium.org>
Date: Thu Jul 06 04:50:25 2017

Revert of Make OS audio buffer size limits visible. (patchset #13 id:240001 of https://codereview.chromium.org/2908073002/ )

Reason for revert:
The reason for reverting is: New test is asserting on Linux GPU bots (physical
machines); see  http://crbug.com/708917  .

Original issue's description:
> Make OS audio buffer size limits visible.
>
> Make platform-specific audio buffer size limits visible in limits.h and update AudioLatency::GetExactBufferSize() to allow requesting buffer sizes down to the minimum. Update OSX and CRAS to allow audio buffer sizes below their previous minimums when using GetExactBufferSize(), for use in Web Audio AudioContext creation with a latencyHint.
>
> BUG= 708917 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
>
> Review-Url: https://codereview.chromium.org/2908073002
> Cr-Commit-Position: refs/heads/master@{#484231}
> Committed: https://chromium.googlesource.com/chromium/src/+/3fe2409358437193a07496c2d0a0b559ef399760

TBR=mkwst@chromium.org,dalecurtis@chromium.org,hongchan@chromium.org,olka@chromium.org,peter@chromium.org,rtoy@chromium.org,andrew.macpherson@soundtrap.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 708917 

Review-Url: https://codereview.chromium.org/2973773002
Cr-Commit-Position: refs/heads/master@{#484467}

[modify] https://crrev.com/0214722083d86f16e50a01d6908c8b3c2f1b8c78/content/renderer/media/renderer_webaudiodevice_impl.cc
[modify] https://crrev.com/0214722083d86f16e50a01d6908c8b3c2f1b8c78/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
[modify] https://crrev.com/0214722083d86f16e50a01d6908c8b3c2f1b8c78/media/BUILD.gn
[modify] https://crrev.com/0214722083d86f16e50a01d6908c8b3c2f1b8c78/media/audio/audio_manager_unittest.cc
[modify] https://crrev.com/0214722083d86f16e50a01d6908c8b3c2f1b8c78/media/audio/cras/audio_manager_cras.cc
[modify] https://crrev.com/0214722083d86f16e50a01d6908c8b3c2f1b8c78/media/audio/mac/audio_manager_mac.cc
[modify] https://crrev.com/0214722083d86f16e50a01d6908c8b3c2f1b8c78/media/audio/pulse/audio_manager_pulse.cc
[modify] https://crrev.com/0214722083d86f16e50a01d6908c8b3c2f1b8c78/media/base/audio_latency.cc
[modify] https://crrev.com/0214722083d86f16e50a01d6908c8b3c2f1b8c78/media/base/limits.h
[delete] https://crrev.com/afd27c28465fe877c97f3c6a0afb96eeff36fce8/media/base/mac/audio_latency_mac.cc
[delete] https://crrev.com/afd27c28465fe877c97f3c6a0afb96eeff36fce8/media/base/mac/audio_latency_mac.h
[modify] https://crrev.com/0214722083d86f16e50a01d6908c8b3c2f1b8c78/third_party/WebKit/Source/platform/audio/AudioDestination.cpp

Hi kbr@, if it would be possible to have the audio_unittests run as part of linux_chromium_rel_ng (as per your comment #6) that would be great! I'm looking into the failure now but as I don't have a Linux machine here I'll have to rely on the CQ machine tests passing to ensure that I've resolved the issue. Let me know if that could work and thanks for the help!

Yeah I thought we got this test as part of "master.tryserver.chromium.linux:linux_optional_gpu_tests_rel" -- @kbr, does this not run on the GPU bots that we add for media/audio changes?

Sorry about that, it was an oversight that these were running only on the FYI waterfall and not also the optional tryservers. Fixing in https://chromium-review.googlesource.com/c/563077/ .

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0a4bc5d41049c519193b922b67777f4efb3870fd

commit 0a4bc5d41049c519193b922b67777f4efb3870fd
Author: andrew.macpherson <andrew.macpherson@soundtrap.com>
Date: Fri Jul 07 12:33:05 2017

Make OS audio buffer size limits visible.

Make platform-specific audio buffer size limits visible in limits.h and update AudioLatency::GetExactBufferSize() to allow requesting buffer sizes down to the minimum. Update OSX and CRAS to allow audio buffer sizes below their previous minimums when using GetExactBufferSize(), for use in Web Audio AudioContext creation with a latencyHint.

BUG= 708917 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2908073002
Cr-Original-Commit-Position: refs/heads/master@{#484231}
Committed: https://chromium.googlesource.com/chromium/src/+/3fe2409358437193a07496c2d0a0b559ef399760
Review-Url: https://codereview.chromium.org/2908073002
Cr-Commit-Position: refs/heads/master@{#484895}

[modify] https://crrev.com/0a4bc5d41049c519193b922b67777f4efb3870fd/content/renderer/media/renderer_webaudiodevice_impl.cc
[modify] https://crrev.com/0a4bc5d41049c519193b922b67777f4efb3870fd/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
[modify] https://crrev.com/0a4bc5d41049c519193b922b67777f4efb3870fd/media/BUILD.gn
[modify] https://crrev.com/0a4bc5d41049c519193b922b67777f4efb3870fd/media/audio/audio_manager_unittest.cc
[modify] https://crrev.com/0a4bc5d41049c519193b922b67777f4efb3870fd/media/audio/cras/audio_manager_cras.cc
[modify] https://crrev.com/0a4bc5d41049c519193b922b67777f4efb3870fd/media/audio/mac/audio_manager_mac.cc
[modify] https://crrev.com/0a4bc5d41049c519193b922b67777f4efb3870fd/media/base/audio_latency.cc
[modify] https://crrev.com/0a4bc5d41049c519193b922b67777f4efb3870fd/media/base/limits.h
[add] https://crrev.com/0a4bc5d41049c519193b922b67777f4efb3870fd/media/base/mac/audio_latency_mac.cc
[add] https://crrev.com/0a4bc5d41049c519193b922b67777f4efb3870fd/media/base/mac/audio_latency_mac.h
[modify] https://crrev.com/0a4bc5d41049c519193b922b67777f4efb3870fd/third_party/WebKit/Source/platform/audio/AudioDestination.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/047f568242e113879ed4523c3f6f5ec31054f92f

commit 047f568242e113879ed4523c3f6f5ec31054f92f
Author: Kenneth Russell <kbr@chromium.org>
Date: Fri Jul 07 17:40:59 2017

Run audio_unittests on optional GPU tryservers.

These will now run on win_optional_gpu_tests_rel,
linux_optional_gpu_tests_rel, and mac_optional_gpu_tests_rel, on
physical hardware. Previously they'd only been running on the
chromium.gpu.fyi waterfall.

BUG= 708917 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Change-Id: Ifc4e6ab582324d7217793f0137d5df57cd7f1a11
Reviewed-on: https://chromium-review.googlesource.com/563077
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484969}
[modify] https://crrev.com/047f568242e113879ed4523c3f6f5ec31054f92f/content/test/gpu/generate_buildbot_json.py
[modify] https://crrev.com/047f568242e113879ed4523c3f6f5ec31054f92f/testing/buildbot/chromium.gpu.fyi.json

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

I think this can be closed now, the min/max (for platforms that have fixed limits) were moved to limits.h by the commit in #c11.

Fixed per c#16.