Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2715893002:
Fixed build error when rtc_use_dummy_audio_file_devices=true (Closed)
DescriptionRTC_DCHECK() is used in place of assert().
BUG=webrtc:7222
Review-Url: https://codereview.webrtc.org/2715893002
Cr-Commit-Position: refs/heads/master@{#17686}
Committed: https://chromium.googlesource.com/external/webrtc/+/1946be4bb20bf92774e8e3f3b8fb4f58c802ca3e
Patch Set 1 : Added #include <cassert> #Patch Set 2 : Use RTC_DCHECK() instead of assert() #Messages
Total messages: 26 (16 generated)
The CQ bit was checked by hillma@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Added #include <cassert> in file_audio_device_factory.cc BUG=webrtc:7222 ========== to ========== Added #include <cassert> in file_audio_device_factory.cc BUG=webrtc:7222 NOTRY=True TBR=henrika@webrtc.org ==========
kjellander@webrtc.org changed reviewers: + henrika@webrtc.org
Description was changed from ========== Added #include <cassert> in file_audio_device_factory.cc BUG=webrtc:7222 NOTRY=True TBR=henrika@webrtc.org ========== to ========== Added #include <cassert> in file_audio_device_factory.cc BUG=webrtc:7222 NOTRY=True ==========
kjellander@webrtc.org changed reviewers: + kjellander@webrtc.org
Thanks. I'll have a look at setting up our bots to build this configuration so we catch future similar problems. lgtm but you need henrika@ as well
On 2017/02/28 06:16:52, kjellander_webrtc wrote: > Thanks. I'll have a look at setting up our bots to build this configuration so > we catch future similar problems. > lgtm but you need henrika@ as well is rtc_use_dummy_audio_file_devices being used? btw, instead of using assert(), we should be using RTC_DCHECK.
On 2017/02/28 08:14:02, tommi (webrtc) wrote: > On 2017/02/28 06:16:52, kjellander_webrtc wrote: > > Thanks. I'll have a look at setting up our bots to build this configuration so > > we catch future similar problems. > > lgtm but you need henrika@ as well > > is rtc_use_dummy_audio_file_devices being used? I believe it's used by internal projects. > btw, instead of using assert(), we should be using RTC_DCHECK. That's a good idea, let's do that instead.
Description was changed from ========== Added #include <cassert> in file_audio_device_factory.cc BUG=webrtc:7222 NOTRY=True ========== to ========== RTC_DCHECK() is used in place of assert(). BUG=webrtc:7222 NOTRY=True ==========
The support was originally added in https://webrtc-codereview.appspot.com/19729004. "By specifying the define WEBRTC_DUMMY_FILE_DEVICES (which is similar to WEBRTC_DUMMY_AUDIO_BUILD) an application will be able to tell WebRTC to play out audio to a file and feed audio in from a file. We want to do so we can better test WebRTC-using applications by recording what the audio stack outputs and feeding known audio in for quality tests." hillma, could you perhaps add some details on why you need this support as well. lgtm
LGTM again. This time as henrika@webrtc.org
On 2017/03/02 08:28:50, henrika_webrtc wrote: > LGTM again. This time as mailto:henrika@webrtc.org I want it because I'm using WebRTC on devices that haven't a working AudioDeviceModule/AudioDevice yet. Having a file dummy device allows to me test out network aspects before "real" working audio.
Ack
I'm going to submit this to unblock https://chromium-review.googlesource.com/c/447189/ and https://bugs.chromium.org/p/webrtc/issues/detail?id=7222
The CQ bit was checked by kjellander@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from kjellander@webrtc.org Link to the patchset: https://codereview.webrtc.org/2715893002/#ps20001 (title: "Use RTC_DCHECK() instead of assert()")
The CQ bit was unchecked by kjellander@webrtc.org
Description was changed from ========== RTC_DCHECK() is used in place of assert(). BUG=webrtc:7222 NOTRY=True ========== to ========== RTC_DCHECK() is used in place of assert(). BUG=webrtc:7222 ==========
The CQ bit was checked by kjellander@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492062548429460,
"parent_rev": "38c65c8fb481e2ee209ebc1b52460894787f7ae3", "commit_rev":
"1946be4bb20bf92774e8e3f3b8fb4f58c802ca3e"}
Message was sent while issue was closed.
Description was changed from ========== RTC_DCHECK() is used in place of assert(). BUG=webrtc:7222 ========== to ========== RTC_DCHECK() is used in place of assert(). BUG=webrtc:7222 Review-Url: https://codereview.webrtc.org/2715893002 Cr-Commit-Position: refs/heads/master@{#17686} Committed: https://chromium.googlesource.com/external/webrtc/+/1946be4bb20bf92774e8e3f3b... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/external/webrtc/+/1946be4bb20bf92774e8e3f3b... |
