Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2534843002:
Added support for changing the volume of RTCAudioSource as discussed in BUG=webrtc:6533 (Closed)
DescriptionAdded support for changing the volume of RTCAudioSource as discussed in BUG=webrtc:6533
This is a short term solution to change the volume of a RTCAudioTrack (which contains an RTCAudioSource property) until applyConstraints for RTCMediaStreamTracks has been implemented.
This CL adds one new Objective-C method to AudioSourceInterface's wrapper: -(void)setVolume:(double)volume
BUG=webrtc:6533, webrtc:6805
This is my first CL for Chromium/WebRTC, so please let me know if I did something wrong.
Review-Url: https://codereview.webrtc.org/2534843002
Cr-Commit-Position: refs/heads/master@{#16809}
Committed: https://chromium.googlesource.com/external/webrtc/+/0d1305ee8873d4ae3d6a0401056cb3dab78fc390
Patch Set 1 #Patch Set 2 : use @property for changing volume of RTCAudioSource #
Total comments: 7
Patch Set 3 : implemented suggested changes by tkchin_webrtc #
Total comments: 6
Patch Set 4 : Comment regarding temporary API is now more clear, added suggestions from discussion #
Total comments: 1
Patch Set 5 : Added support for changing the volume of RTCAudioSource as discussed in BUG=webrtc:6533 #Patch Set 6 : added manual @syntheziser for volume-property #
Messages
Total messages: 44 (21 generated)
Description was changed from ========== Added support for changing the volume of RTCAudioSource as discussed in BUG=webrtc:6533 This is a short term solution to change the volume of a RTCAudioTrack (which contains an RTCAudioSource property) until applyConstraints for RTCMediaStreamTracks has been implemented. This CL adds one new Objective-C method to AudioSourceInterface's wrapper: -(void)setVolume:(double)volume BUG=webrtc:6533 ========== to ========== Added support for changing the volume of RTCAudioSource as discussed in BUG=webrtc:6533 This is a short term solution to change the volume of a RTCAudioTrack (which contains an RTCAudioSource property) until applyConstraints for RTCMediaStreamTracks has been implemented. This CL adds one new Objective-C method to AudioSourceInterface's wrapper: -(void)setVolume:(double)volume BUG=webrtc:6533 This is my first CL for Chromium/WebRTC, so please let me know if I did something wrong. ==========
frederik.riedel@frogg.io changed reviewers: + pthatcher@google.com, solenberg@webrtc.org, tkchin@chromium.org
I'm not an that familiar with objc, but shouldn't this be implemented using a property instead. Don't take my word for it though - tkchin@?
tkchin@webrtc.org changed reviewers: + tkchin@webrtc.org
https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Classes/RTCAudioSource.mm (right): https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCAudioSource.mm:41: - (void)setVolume:(double) volume { no space after (double) https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCAudioSource.mm:42: _volume = volume; nit: 2 spaces not 4 (see other indentation in file) https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h (right): https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h:23: @property(nonatomic) double volume; nit: nonatomic, assign doco that this is temporary, link to a bug with plans for future fix doco what the valid range is, what happens if out of bounds
On 2016/11/28 21:29:54, tkchin_webrtc wrote: > https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... > File webrtc/sdk/objc/Framework/Classes/RTCAudioSource.mm (right): > > https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... > webrtc/sdk/objc/Framework/Classes/RTCAudioSource.mm:41: - > (void)setVolume:(double) volume { > no space after (double) > > https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... > webrtc/sdk/objc/Framework/Classes/RTCAudioSource.mm:42: _volume = volume; > nit: 2 spaces not 4 (see other indentation in file) > > https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... > File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h (right): > > https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... > webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h:23: > @property(nonatomic) double volume; > nit: nonatomic, assign > > doco that this is temporary, link to a bug with plans for future fix > > doco what the valid range is, what happens if out of bounds I'm not sure this is the right thing to do. This may make sense for a remote audio source, but what does it mean for a local audio source?
On 2016/11/28 21:30:35, tkchin_webrtc wrote: > On 2016/11/28 21:29:54, tkchin_webrtc wrote: > > > https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... > > File webrtc/sdk/objc/Framework/Classes/RTCAudioSource.mm (right): > > > > > https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... > > webrtc/sdk/objc/Framework/Classes/RTCAudioSource.mm:41: - > > (void)setVolume:(double) volume { > > no space after (double) > > > > > https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... > > webrtc/sdk/objc/Framework/Classes/RTCAudioSource.mm:42: _volume = volume; > > nit: 2 spaces not 4 (see other indentation in file) > > > > > https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... > > File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h (right): > > > > > https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... > > webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h:23: > > @property(nonatomic) double volume; > > nit: nonatomic, assign > > > > doco that this is temporary, link to a bug with plans for future fix > > > > doco what the valid range is, what happens if out of bounds > > I'm not sure this is the right thing to do. This may make sense for a remote > audio source, but what does it mean for a local audio source? I implemented your suggested changes, thanks for pointing them out. However there are still two things missing: a specific bug report for implementing applyConstraints() as suggested by solenberg in BUG=webrtc:6533#c14 and an owner of the //TODO: within the code
solenberg@webrtc.org changed reviewers: + hta@webrtc.org
Adding Harald to resolve the interface discussion: - Harald, do you know what the plans are re: MediaStreamTrack.applyConstraints()? Is it better to avoid the temporary solution and land that? - If applyConstraints() is not going to happen soon, there shouldn't be any problems adding the property to RTCAudioSource from the viewpoint of deviating from any spec. Correct me if I'm wrong. (Whether we want to take on the maintenance burden of a temp API is another question.) https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h (right): https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h:23: @property(nonatomic) double volume; On 2016/11/28 21:29:54, tkchin_webrtc wrote: > nit: nonatomic, assign > > doco that this is temporary, link to a bug with plans for future fix > > doco what the valid range is, what happens if out of bounds Currently SetVolume() is a no-op for local streams. https://codereview.webrtc.org/2534843002/diff/40001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h (right): https://codereview.webrtc.org/2534843002/diff/40001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h:25: // TODO: implementing support for applyConstraints() to MediaStreamTrack will TODO should have an owner
Description was changed from ========== Added support for changing the volume of RTCAudioSource as discussed in BUG=webrtc:6533 This is a short term solution to change the volume of a RTCAudioTrack (which contains an RTCAudioSource property) until applyConstraints for RTCMediaStreamTracks has been implemented. This CL adds one new Objective-C method to AudioSourceInterface's wrapper: -(void)setVolume:(double)volume BUG=webrtc:6533 This is my first CL for Chromium/WebRTC, so please let me know if I did something wrong. ========== to ========== Added support for changing the volume of RTCAudioSource as discussed in BUG=webrtc:6533 This is a short term solution to change the volume of a RTCAudioTrack (which contains an RTCAudioSource property) until applyConstraints for RTCMediaStreamTracks has been implemented. This CL adds one new Objective-C method to AudioSourceInterface's wrapper: -(void)setVolume:(double)volume BUG=webrtc:6533, webrtc:6805 This is my first CL for Chromium/WebRTC, so please let me know if I did something wrong. ==========
Turns out the job of adding the proper wiring to the native APIs may not be *that* big, see webrtc:6805. hta@, pthatcher@ - is work on that issue something that lies in the near future? On 2016/11/30 11:38:04, the sun wrote: > Adding Harald to resolve the interface discussion: > > - Harald, do you know what the plans are re: > MediaStreamTrack.applyConstraints()? Is it better to avoid the temporary > solution and land that? > > - If applyConstraints() is not going to happen soon, there shouldn't be any > problems adding the property to RTCAudioSource from the viewpoint of deviating > from any spec. Correct me if I'm wrong. (Whether we want to take on the > maintenance burden of a temp API is another question.) > > https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... > File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h (right): > > https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... > webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h:23: > @property(nonatomic) double volume; > On 2016/11/28 21:29:54, tkchin_webrtc wrote: > > nit: nonatomic, assign > > > > doco that this is temporary, link to a bug with plans for future fix > > > > doco what the valid range is, what happens if out of bounds > > Currently SetVolume() is a no-op for local streams. > > https://codereview.webrtc.org/2534843002/diff/40001/webrtc/sdk/objc/Framework... > File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h (right): > > https://codereview.webrtc.org/2534843002/diff/40001/webrtc/sdk/objc/Framework... > webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h:25: // TODO: > implementing support for applyConstraints() to MediaStreamTrack will > TODO should have an owner
lgtm https://codereview.webrtc.org/2534843002/diff/40001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h (right): https://codereview.webrtc.org/2534843002/diff/40001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h:26: // replace this implementation I think the surface exposed by webrtc should have a volume property and/or a setvolume method. The constraints concept needs to live in Chromium alone. But (0..10) is an odd value range, and the nature of the relationship to the volume is not specified, so the API needs to be temporary. So leaving a TODO is nice. (ref discussion with @thesun.
lgtm % comments https://codereview.webrtc.org/2534843002/diff/40001/AUTHORS File AUTHORS (right): https://codereview.webrtc.org/2534843002/diff/40001/AUTHORS#newcode18 AUTHORS:18: Hugues Ekra <hekra01@gmail.com> Who is this? https://codereview.webrtc.org/2534843002/diff/40001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h (right): https://codereview.webrtc.org/2534843002/diff/40001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h:23: // Sets the volume to the RTCMediaSource. |volume| is in the range of [0, 10]. Make that read: "Sets the volume for the RTCMediaSource. |volume] is a gain value in the range [0, 10]. https://codereview.webrtc.org/2534843002/diff/40001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h:26: // replace this implementation On 2016/12/01 13:59:15, hta-webrtc wrote: > I think the surface exposed by webrtc should have a volume property and/or a > setvolume method. The constraints concept needs to live in Chromium alone. But > (0..10) is an odd value range, and the nature of the relationship to the volume > is not specified, so the API needs to be temporary. So leaving a TODO is nice. > > (ref discussion with @thesun. The [0..10] range stems from a DCHECK in RemoteAudioSource which appears arbitrary but not unreasonable. Lower layers accept any gain value and a gain of 10 gives a 20dB boost, which should be enough for many uses. For a temp API I think we can leave it like that.
Implemented suggested changes from discussion, especially regarding the documentation. https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Classes/RTCAudioSource.mm (right): https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCAudioSource.mm:41: - (void)setVolume:(double) volume { On 2016/11/28 21:29:54, tkchin_webrtc wrote: > no space after (double) Done. https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCAudioSource.mm:42: _volume = volume; On 2016/11/28 21:29:54, tkchin_webrtc wrote: > nit: 2 spaces not 4 (see other indentation in file) Done. https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h (right): https://codereview.webrtc.org/2534843002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h:23: @property(nonatomic) double volume; On 2016/11/28 21:29:54, tkchin_webrtc wrote: > nit: nonatomic, assign > > doco that this is temporary, link to a bug with plans for future fix > > doco what the valid range is, what happens if out of bounds Done. https://codereview.webrtc.org/2534843002/diff/40001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h (right): https://codereview.webrtc.org/2534843002/diff/40001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Headers/WebRTC/RTCAudioSource.h:23: // Sets the volume to the RTCMediaSource. |volume| is in the range of [0, 10]. On 2016/12/01 18:24:19, the sun wrote: > Make that read: > > "Sets the volume for the RTCMediaSource. |volume] is a gain value in the range > [0, 10]. Done.
lgtm https://codereview.webrtc.org/2534843002/diff/60001/AUTHORS File AUTHORS (right): https://codereview.webrtc.org/2534843002/diff/60001/AUTHORS#newcode1 AUTHORS:1: # Names should be added to this file like so: what changed in this line?
solenberg@webrtc.org changed reviewers: - pthatcher@google.com, tkchin@chromium.org
On 2016/12/02 21:26:36, tkchin_webrtc wrote: > lgtm > > https://codereview.webrtc.org/2534843002/diff/60001/AUTHORS > File AUTHORS (right): > > https://codereview.webrtc.org/2534843002/diff/60001/AUTHORS#newcode1 > AUTHORS:1: # Names should be added to this file like so: > what changed in this line? I was prompted to look at this again, because someone implemented something similar on Android: https://codereview.webrtc.org/2710683009 What is the next step for this CL?
On 2017/02/22 21:52:57, polarapps.feedback wrote: > On 2016/12/02 21:26:36, tkchin_webrtc wrote: > > lgtm > > > > https://codereview.webrtc.org/2534843002/diff/60001/AUTHORS > > File AUTHORS (right): > > > > https://codereview.webrtc.org/2534843002/diff/60001/AUTHORS#newcode1 > > AUTHORS:1: # Names should be added to this file like so: > > what changed in this line? > > I was prompted to look at this again, because someone implemented something > similar on Android: > https://codereview.webrtc.org/2710683009 > > What is the next step for this CL? It looks like the author only needs to rebase and commit it.
The CQ bit was checked by frederik.riedel@frogg.io 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: Try jobs failed on following builders: mac_asan on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/22300)
The CQ bit was unchecked by frederik.riedel@frogg.io
The CQ bit was checked by frederik.riedel@frogg.io
The patchset sent to the CQ was uploaded after l-g-t-m from solenberg@webrtc.org, hta@webrtc.org Link to the patchset: https://codereview.webrtc.org/2534843002/#ps60001 (title: "Comment regarding temporary API is now more clear, added suggestions from discussion")
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
Try jobs failed on following builders: ios32_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_ios9_dbg/buil...) ios_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/21908) mac_asan on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/22302)
The CQ bit was checked by frederik.riedel@frogg.io
The patchset sent to the CQ was uploaded after l-g-t-m from solenberg@webrtc.org, hta@webrtc.org, tkchin@webrtc.org Link to the patchset: https://codereview.webrtc.org/2534843002/#ps80001 (title: "Added support for changing the volume of RTCAudioSource as discussed in BUG=webrtc:6533")
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
Try jobs failed on following builders: mac_baremetal on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/19141)
On 2017/02/23 13:14:25, commit-bot: I haz the power wrote: > CQ is trying da patch. Follow status at > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/... Thanks for reminding me to take action. I've rebased my commits and uploaded them (patch set 5), but there seems to be some problem. I am not sture why the build servers report errors. What do you suggest to do in this case? (Maybe I've done something wrong).
On 2017/02/23 13:21:49, Frederik wrote: > On 2017/02/23 13:14:25, commit-bot: I haz the power wrote: > > CQ is trying da patch. Follow status at > > > > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/... > > Thanks for reminding me to take action. > I've rebased my commits and uploaded them (patch set 5), but there seems to be > some problem. I am not sture why the build servers report errors. > > What do you suggest to do in this case? (Maybe I've done something wrong). Click on the build bots to find out what is failing. In this case it looks like an ObjC-specific compile error, at which I'm at a loss at resolving for you. Perhaps something has changed in a build config. Maybe you can figure it out yourself, else ping tkchin_webrtc, who will no doubt have an idea.
The CQ bit was checked by frederik.riedel@frogg.io
The patchset sent to the CQ was uploaded after l-g-t-m from solenberg@webrtc.org, hta@webrtc.org, tkchin@webrtc.org Link to the patchset: https://codereview.webrtc.org/2534843002/#ps100001 (title: "added manual @syntheziser for volume-property")
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": 100001, "attempt_start_ts": 1487884499925210,
"parent_rev": "7aadbfa06e7296af0d0e7b4b793ae9f978d24fbc", "commit_rev":
"0d1305ee8873d4ae3d6a0401056cb3dab78fc390"}
Message was sent while issue was closed.
Description was changed from ========== Added support for changing the volume of RTCAudioSource as discussed in BUG=webrtc:6533 This is a short term solution to change the volume of a RTCAudioTrack (which contains an RTCAudioSource property) until applyConstraints for RTCMediaStreamTracks has been implemented. This CL adds one new Objective-C method to AudioSourceInterface's wrapper: -(void)setVolume:(double)volume BUG=webrtc:6533, webrtc:6805 This is my first CL for Chromium/WebRTC, so please let me know if I did something wrong. ========== to ========== Added support for changing the volume of RTCAudioSource as discussed in BUG=webrtc:6533 This is a short term solution to change the volume of a RTCAudioTrack (which contains an RTCAudioSource property) until applyConstraints for RTCMediaStreamTracks has been implemented. This CL adds one new Objective-C method to AudioSourceInterface's wrapper: -(void)setVolume:(double)volume BUG=webrtc:6533, webrtc:6805 This is my first CL for Chromium/WebRTC, so please let me know if I did something wrong. Review-Url: https://codereview.webrtc.org/2534843002 Cr-Commit-Position: refs/heads/master@{#16809} Committed: https://chromium.googlesource.com/external/webrtc/+/0d1305ee8873d4ae3d6a04010... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://chromium.googlesource.com/external/webrtc/+/0d1305ee8873d4ae3d6a04010...
Message was sent while issue was closed.
On 2017/02/23 21:57:06, commit-bot: I haz the power wrote: > Committed patchset #6 (id:100001) as > https://chromium.googlesource.com/external/webrtc/+/0d1305ee8873d4ae3d6a04010... This CL removes someone from the AUTHORS file, why? Also, it has a TODO with no owner. It should probably not have landed in it's current state.
Message was sent while issue was closed.
On 2017/03/23 15:10:46, kthelgason wrote: > On 2017/02/23 21:57:06, commit-bot: I haz the power wrote: > > Committed patchset #6 (id:100001) as > > > https://chromium.googlesource.com/external/webrtc/+/0d1305ee8873d4ae3d6a04010... > > This CL removes someone from the AUTHORS file, why? Also, it has a TODO with no > owner. It should probably not have landed in it's current state. You're right. I wonder what happened. I'll fix it since I should've kept my eyes open during review.
Message was sent while issue was closed.
On 2017/03/23 16:49:26, the sun wrote: > On 2017/03/23 15:10:46, kthelgason wrote: > > On 2017/02/23 21:57:06, commit-bot: I haz the power wrote: > > > Committed patchset #6 (id:100001) as > > > > > > https://chromium.googlesource.com/external/webrtc/+/0d1305ee8873d4ae3d6a04010... > > > > This CL removes someone from the AUTHORS file, why? Also, it has a TODO with > no > > owner. It should probably not have landed in it's current state. > > You're right. I wonder what happened. I'll fix it since I should've kept my eyes > open during review. I'm sorry it was not intended by me to delete someone from the Authors file. Thanks for fixing the issue, sun! |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
