New issue
Advanced search Search tips

Issue 802212 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Specify time units (us) in time members of media::Audio[In|Out]putBufferParameters

Project Member Reported by olka@chromium.org, Jan 16 2018

Issue description

See https://cs.chromium.org/chromium/src/media/base/audio_parameters.h.
It should be
AudioInputBufferParameters::capture_time_us;
AudioOutputBufferParameters::delay_us;
AudioOutputBufferParameters::delay_timestamp_us;

The values are converted to/from microseconds when being passed over IPC.
It's really hard to read the code there without having a clear indication of time units. And a future change of time units may also result in inconsistent interpretation of the values in different parts of the code.

That can lead to pretty bad hard-to-catch errors - we've already seen cases when time values were interpreted incorrectly because the units were not indicated by data field names.

 

Comment 1 by olka@chromium.org, Feb 26 2018

Owner: ----
I am interested to take this one as my first bug :)
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 26 2018

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

commit 058bf2fcf128db0a4ca584f666192397741e9de6
Author: Lizhi Fan <lizhi.fan@samsung.com>
Date: Mon Mar 26 14:56:42 2018

Specify time units (us) in time members of
media::Audio[In|Out]putBufferParameters

member name changed to:
AudioInputBufferParameters::capture_time_us;
AudioOutputBufferParameters::delay_us;
AudioOutputBufferParameters::delay_timestamp_us;

R=raymes@chromium.org, tommi@chromium.org

Bug: 802212
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I95b3205087e04b9508760d6bc4cc9fb88456c5b0
Reviewed-on: https://chromium-review.googlesource.com/977249
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Commit-Queue: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545788}
[modify] https://crrev.com/058bf2fcf128db0a4ca584f666192397741e9de6/AUTHORS
[modify] https://crrev.com/058bf2fcf128db0a4ca584f666192397741e9de6/media/audio/audio_input_device.cc
[modify] https://crrev.com/058bf2fcf128db0a4ca584f666192397741e9de6/media/audio/audio_input_sync_writer.cc
[modify] https://crrev.com/058bf2fcf128db0a4ca584f666192397741e9de6/media/audio/audio_output_device.cc
[modify] https://crrev.com/058bf2fcf128db0a4ca584f666192397741e9de6/media/audio/audio_output_device_unittest.cc
[modify] https://crrev.com/058bf2fcf128db0a4ca584f666192397741e9de6/media/audio/audio_sync_reader.cc
[modify] https://crrev.com/058bf2fcf128db0a4ca584f666192397741e9de6/media/audio/win/audio_output_win_unittest.cc
[modify] https://crrev.com/058bf2fcf128db0a4ca584f666192397741e9de6/media/base/audio_parameters.h
[modify] https://crrev.com/058bf2fcf128db0a4ca584f666192397741e9de6/ppapi/proxy/audio_output_resource.cc
[modify] https://crrev.com/058bf2fcf128db0a4ca584f666192397741e9de6/ppapi/shared_impl/ppb_audio_shared.cc

Sign in to add a comment