Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1776353006:
[Chromecast] Add metrics for logging platform A/V bitrate estimates. (Closed)
|
Created:
4 years, 9 months ago by ejason Modified:
4 years, 9 months ago CC:
chromium-reviews, gunsch+watch_chromium.org, lcwu+watch_chromium.org, halliwell+watch_chromium.org, feature-media-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Chromecast] Add metrics for logging platform A/V bitrate estimates.
BUG=internal b/26496758
Committed: https://crrev.com/24fbb39886e90b81175b7fc9f8fc79677a53d176
Cr-Commit-Position: refs/heads/master@{#380787}
Patch Set 1 #Patch Set 2 : Fix issue with handling measured bitrate values of 0 #Patch Set 3 : Fix formatting issue #
Total comments: 10
Patch Set 4 : Refactored to correctly handle state transitions and eliminate duplication #
Total comments: 2
Patch Set 5 : Add const qualifier to bytes_decoded accessor #
Total comments: 12
Patch Set 6 : clean up handling of tracking initial sample time #Patch Set 7 : remove unnecessary calls to ResetBitrateState() #
Total comments: 4
Patch Set 8 : Remove unnecessary initialization #
Total comments: 4
Patch Set 9 : Handle different audio/video pipeline configurations #
Messages
Total messages: 31 (7 generated)
Description was changed from ========== [Chromecast] Add metrics for logging platform A/V bitrate estimates. BUG=internal b/26496758 ========== to ========== [Chromecast] Add metrics for logging platform A/V bitrate estimates. BUG=internal b/26496758 ==========
jasonroberts@google.com changed reviewers: + kmackay@chromium.org
jasonroberts@google.com changed reviewers: + halliwell@chromium.org
On 2016/03/10 02:04:03, ejason wrote: > mailto:jasonroberts@google.com changed reviewers: > + mailto:halliwell@chromium.org Metrics for estimated audio/video bitrate as seen by the platform. Events defined here: https://docs.google.com/document/d/1XCJaSRBP-5EfmFh-JN0-vGs3R-F3oWE4_pMKgQn2_...
On 2016/03/10 02:05:29, ejason wrote: > On 2016/03/10 02:04:03, ejason wrote: > > mailto:jasonroberts@google.com changed reviewers: > > + mailto:halliwell@chromium.org > > Metrics for estimated audio/video bitrate as seen by the platform. Events > defined here: > > https://docs.google.com/document/d/1XCJaSRBP-5EfmFh-JN0-vGs3R-F3oWE4_pMKgQn2_... This is from your google account, can you submit from chromium.org user?
On 2016/03/10 14:47:11, halliwell wrote: > On 2016/03/10 02:05:29, ejason wrote: > > On 2016/03/10 02:04:03, ejason wrote: > > > mailto:jasonroberts@google.com changed reviewers: > > > + mailto:halliwell@chromium.org > > > > Metrics for estimated audio/video bitrate as seen by the platform. Events > > defined here: > > > > > https://docs.google.com/document/d/1XCJaSRBP-5EfmFh-JN0-vGs3R-F3oWE4_pMKgQn2_... > > This is from your google account, can you submit from http://chromium.org user? I just tried to log in but it looks like I need to enable 2-factor authentication on my account (it's been a while since I used it): https://screenshot.googleplex.com/z1aEDTfFJSL Do you know who I should talk to to enable this?
On 2016/03/10 17:22:54, ejason wrote: > On 2016/03/10 14:47:11, halliwell wrote: > > On 2016/03/10 02:05:29, ejason wrote: > > > On 2016/03/10 02:04:03, ejason wrote: > > > > mailto:jasonroberts@google.com changed reviewers: > > > > + mailto:halliwell@chromium.org > > > > > > Metrics for estimated audio/video bitrate as seen by the platform. Events > > > defined here: > > > > > > > > > https://docs.google.com/document/d/1XCJaSRBP-5EfmFh-JN0-vGs3R-F3oWE4_pMKgQn2_... > > > > This is from your google account, can you submit from http://chromium.org > user? > > I just tried to log in but it looks like I need to enable 2-factor > authentication on my account (it's been a while since I used it): > > https://screenshot.googleplex.com/z1aEDTfFJSL > > Do you know who I should talk to to enable this? Also, I set my preferences in git, so the git commit message looks like: commit d0a66a00114f07b260db06e892166fa9397aa857 Author: ejason <jasonroberts@chromium.org> Date: Wed Mar 9 17:09:03 2016 -0800 [Chromecast] Add metrics for logging platform A/V bitrate estimates. BUG=internal b/26496758
On 2016/03/10 17:23:44, ejason wrote: > On 2016/03/10 17:22:54, ejason wrote: > > On 2016/03/10 14:47:11, halliwell wrote: > > > On 2016/03/10 02:05:29, ejason wrote: > > > > On 2016/03/10 02:04:03, ejason wrote: > > > > > mailto:jasonroberts@google.com changed reviewers: > > > > > + mailto:halliwell@chromium.org > > > > > > > > Metrics for estimated audio/video bitrate as seen by the platform. Events > > > > defined here: > > > > > > > > > > > > > > https://docs.google.com/document/d/1XCJaSRBP-5EfmFh-JN0-vGs3R-F3oWE4_pMKgQn2_... > > > > > > This is from your google account, can you submit from http://chromium.org > > user? > > > > I just tried to log in but it looks like I need to enable 2-factor > > authentication on my account (it's been a while since I used it): > > > > https://screenshot.googleplex.com/z1aEDTfFJSL > > > > Do you know who I should talk to to enable this? > > Also, I set my preferences in git, so the git commit message looks like: > > commit d0a66a00114f07b260db06e892166fa9397aa857 > Author: ejason <mailto:jasonroberts@chromium.org> > Date: Wed Mar 9 17:09:03 2016 -0800 > > [Chromecast] Add metrics for logging platform A/V bitrate estimates. > > BUG=internal b/26496758 I've sent a mail to accounts@chromium.org to try to resolve my login issue.
On 2016/03/10 17:36:08, ejason wrote: > On 2016/03/10 17:23:44, ejason wrote: > > On 2016/03/10 17:22:54, ejason wrote: > > > On 2016/03/10 14:47:11, halliwell wrote: > > > > On 2016/03/10 02:05:29, ejason wrote: > > > > > On 2016/03/10 02:04:03, ejason wrote: > > > > > > mailto:jasonroberts@google.com changed reviewers: > > > > > > + mailto:halliwell@chromium.org > > > > > > > > > > Metrics for estimated audio/video bitrate as seen by the platform. > Events > > > > > defined here: > > > > > > > > > > > > > > > > > > > > https://docs.google.com/document/d/1XCJaSRBP-5EfmFh-JN0-vGs3R-F3oWE4_pMKgQn2_... > > > > > > > > This is from your google account, can you submit from http://chromium.org > > > user? > > > > > > I just tried to log in but it looks like I need to enable 2-factor > > > authentication on my account (it's been a while since I used it): > > > > > > https://screenshot.googleplex.com/z1aEDTfFJSL > > > > > > Do you know who I should talk to to enable this? > > > > Also, I set my preferences in git, so the git commit message looks like: > > > > commit d0a66a00114f07b260db06e892166fa9397aa857 > > Author: ejason <mailto:jasonroberts@chromium.org> > > Date: Wed Mar 9 17:09:03 2016 -0800 > > > > [Chromecast] Add metrics for logging platform A/V bitrate estimates. > > > > BUG=internal b/26496758 > > I've sent a mail to mailto:accounts@chromium.org to try to resolve my login issue. You will need to file a bug against the Chromium infra team to unlock your chromium account. See the announcement here https://groups.google.com/a/chromium.org/forum/#!search/2-step/infra-dev/iOEb... However, using the google account is OK. (I remember seeing a Chromium doc saying that you don't need to have a Chromium account in order to contribute.) Using a non-Chromium account is more of a problem for a reviewer who needs to do owner approval with his/her Chromium account.
On 2016/03/10 17:43:46, lcwu1 wrote: > On 2016/03/10 17:36:08, ejason wrote: > > On 2016/03/10 17:23:44, ejason wrote: > > > On 2016/03/10 17:22:54, ejason wrote: > > > > On 2016/03/10 14:47:11, halliwell wrote: > > > > > On 2016/03/10 02:05:29, ejason wrote: > > > > > > On 2016/03/10 02:04:03, ejason wrote: > > > > > > > mailto:jasonroberts@google.com changed reviewers: > > > > > > > + mailto:halliwell@chromium.org > > > > > > > > > > > > Metrics for estimated audio/video bitrate as seen by the platform. > > Events > > > > > > defined here: > > > > > > > > > > > > > > > > > > > > > > > > > > > https://docs.google.com/document/d/1XCJaSRBP-5EfmFh-JN0-vGs3R-F3oWE4_pMKgQn2_... > > > > > > > > > > This is from your google account, can you submit from > http://chromium.org > > > > user? > > > > > > > > I just tried to log in but it looks like I need to enable 2-factor > > > > authentication on my account (it's been a while since I used it): > > > > > > > > https://screenshot.googleplex.com/z1aEDTfFJSL > > > > > > > > Do you know who I should talk to to enable this? > > > > > > Also, I set my preferences in git, so the git commit message looks like: > > > > > > commit d0a66a00114f07b260db06e892166fa9397aa857 > > > Author: ejason <mailto:jasonroberts@chromium.org> > > > Date: Wed Mar 9 17:09:03 2016 -0800 > > > > > > [Chromecast] Add metrics for logging platform A/V bitrate estimates. > > > > > > BUG=internal b/26496758 > > > > I've sent a mail to mailto:accounts@chromium.org to try to resolve my login > issue. > > You will need to file a bug against the Chromium infra team to unlock your > chromium account. See the announcement here > > https://groups.google.com/a/chromium.org/forum/#!search/2-step/infra-dev/iOEb... > > However, using the google account is OK. (I remember seeing a Chromium doc > saying that you don't need to have a Chromium account in order to contribute.) > Using a non-Chromium account is more of a problem for a reviewer who needs to do > owner approval with his/her Chromium account. Thanks for the link to the PSA. I filed a bug to enable the account again: https://bugs.chromium.org/p/chromium/issues/detail?id=593794
https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... File chromecast/media/cma/pipeline/audio_pipeline_impl.cc (right): https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/audio_pipeline_impl.cc:29: chromecast::metrics::CastMetricsHelper::GetInstance(); nit, no need for 'chromecast::' https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/audio_pipeline_impl.cc:103: elapsed_time_delta_ += current_time - last_sample_time_; If the first time this function is called, we're in Playing state, then last_sample_time_ is base::Time(), elapsed_time_delta_ will be incorrect and we could log an incorrect stat? https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/audio_pipeline_impl.cc:110: audio_bytes_for_bitrate_estimation_ = 0; shouldn't we reset these things on state transitions? What if we get this sequence: * Gather stats * Pause * Play * Gather stats then we're taking into account a time period when we weren't playing. https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... File chromecast/media/cma/pipeline/audio_pipeline_impl.h (right): https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/audio_pipeline_impl.h:50: base::TimeDelta elapsed_time_delta_; include headers for time classes https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... File chromecast/media/cma/pipeline/video_pipeline_impl.h (right): https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/video_pipeline_impl.h:52: int video_bytes_for_bitrate_estimation_; I was about to leave 'ditto' comments on the video implementation. But then I realised, they both inherit from AvPipelineImpl - is it possible to share the logic up a level in the class hierarchy to avoid duplication?
https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... File chromecast/media/cma/pipeline/audio_pipeline_impl.cc (right): https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/audio_pipeline_impl.cc:29: chromecast::metrics::CastMetricsHelper::GetInstance(); On 2016/03/10 17:57:54, halliwell wrote: > nit, no need for 'chromecast::' Done. https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/audio_pipeline_impl.cc:103: elapsed_time_delta_ += current_time - last_sample_time_; On 2016/03/10 17:57:54, halliwell wrote: > If the first time this function is called, we're in Playing state, then > last_sample_time_ is base::Time(), elapsed_time_delta_ will be incorrect and we > could log an incorrect stat? I've updated the new logic to handle the first-time-the-function-is-called case. https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/audio_pipeline_impl.cc:110: audio_bytes_for_bitrate_estimation_ = 0; On 2016/03/10 17:57:54, halliwell wrote: > shouldn't we reset these things on state transitions? > > What if we get this sequence: > * Gather stats > * Pause > * Play > * Gather stats > > then we're taking into account a time period when we weren't playing. The state transitions are handled in media_pipeline_impl_, so I moved this logic there. That also addresses the point on duplication. https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... File chromecast/media/cma/pipeline/audio_pipeline_impl.h (right): https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/audio_pipeline_impl.h:50: base::TimeDelta elapsed_time_delta_; On 2016/03/10 17:57:54, halliwell wrote: > include headers for time classes Done. https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... File chromecast/media/cma/pipeline/video_pipeline_impl.h (right): https://codereview.chromium.org/1776353006/diff/30001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/video_pipeline_impl.h:52: int video_bytes_for_bitrate_estimation_; On 2016/03/10 17:57:54, halliwell wrote: > I was about to leave 'ditto' comments on the video implementation. > > But then I realised, they both inherit from AvPipelineImpl - is it possible to > share the logic up a level in the class hierarchy to avoid duplication? Done. I refactored the code so that this calculation isn't duplicated. I think media_pipeline_impl is the place for this given the current code structure and make sure the calculations are accurate across state transitions.
I'm not sure I trust the new logic either? Sequence A: UpdateMediaTime UpdateStatistics UpdateStatistics UpdateMediaTime => bytes decoded in first UpdateStatistics call are lost? Sequence B: UpdateMediaTime ... UpdateMediaTime => no bytes recorded, even though some may have been decoded, simply because an UpdateStatistics call has not been scheduled https://codereview.chromium.org/1776353006/diff/50001/chromecast/media/cma/pi... File chromecast/media/cma/pipeline/av_pipeline_impl.h (right): https://codereview.chromium.org/1776353006/diff/50001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/av_pipeline_impl.h:59: } nit: const
https://codereview.chromium.org/1776353006/diff/50001/chromecast/media/cma/pi... File chromecast/media/cma/pipeline/av_pipeline_impl.h (right): https://codereview.chromium.org/1776353006/diff/50001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/av_pipeline_impl.h:59: } On 2016/03/11 02:04:32, halliwell wrote: > nit: const Done.
On 2016/03/11 17:33:44, ejason wrote: Duh, previous comments can be ignored. UpdateStatistics is only called from UpdateMediaTime. Taking another look now ... :)
https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... File chromecast/media/cma/pipeline/media_pipeline_impl.cc (right): https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/media_pipeline_impl.cc:61: if (estimated_bitrate_in_kbps > 0) { nit: invert logic if (bitrate <= 0) return; https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/media_pipeline_impl.cc:297: ResetBitrateState(); I would rather call this on transition to the "playing" state (so, when backend->Start() or backend->Resume() is called. Then, you can just record the current time as last_sample_time_ https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/media_pipeline_impl.cc:443: base::Time current_time = base::Time::Now(); Get the current time within the if statement https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/media_pipeline_impl.cc:444: if (!last_sample_time_.is_null() && If you change it to reset just when we enter the playing state, then you don't need to check if the last sample time is null (since it will always be valid whenever we are in the playing state). https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... File chromecast/media/cma/pipeline/media_pipeline_impl.h (right): https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/media_pipeline_impl.h:114: base::Time last_sample_time_; I recommend using base::TimeTicks
https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... File chromecast/media/cma/pipeline/media_pipeline_impl.cc (right): https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/media_pipeline_impl.cc:58: base::StringPiece metric) { + a nit: No need to use StringPiece here - just pass const char*?
https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... File chromecast/media/cma/pipeline/media_pipeline_impl.cc (right): https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/media_pipeline_impl.cc:58: base::StringPiece metric) { On 2016/03/11 18:12:46, halliwell wrote: > + a nit: No need to use StringPiece here - just pass const char*? Done. https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/media_pipeline_impl.cc:61: if (estimated_bitrate_in_kbps > 0) { On 2016/03/11 17:47:40, kmackay wrote: > nit: invert logic > if (bitrate <= 0) > return; Done. https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/media_pipeline_impl.cc:297: ResetBitrateState(); On 2016/03/11 17:47:40, kmackay wrote: > I would rather call this on transition to the "playing" state (so, when > backend->Start() or backend->Resume() is called. Then, you can just record the > current time as last_sample_time_ Thanks this is a really good idea. Done. https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/media_pipeline_impl.cc:443: base::Time current_time = base::Time::Now(); On 2016/03/11 17:47:40, kmackay wrote: > Get the current time within the if statement Done. https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/media_pipeline_impl.cc:444: if (!last_sample_time_.is_null() && On 2016/03/11 17:47:40, kmackay wrote: > If you change it to reset just when we enter the playing state, then you don't > need to check if the last sample time is null (since it will always be valid > whenever we are in the playing state). Done. https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... File chromecast/media/cma/pipeline/media_pipeline_impl.h (right): https://codereview.chromium.org/1776353006/diff/70001/chromecast/media/cma/pi... chromecast/media/cma/pipeline/media_pipeline_impl.h:114: base::Time last_sample_time_; On 2016/03/11 17:47:40, kmackay wrote: > I recommend using base::TimeTicks Done.
lgtm % nits https://codereview.chromium.org/1776353006/diff/110001/chromecast/media/cma/p... File chromecast/media/cma/pipeline/media_pipeline_impl.cc (right): https://codereview.chromium.org/1776353006/diff/110001/chromecast/media/cma/p... chromecast/media/cma/pipeline/media_pipeline_impl.cc:61: if (estimated_bitrate_in_kbps <= 0) { nit: braces not necessary https://codereview.chromium.org/1776353006/diff/110001/chromecast/media/cma/p... chromecast/media/cma/pipeline/media_pipeline_impl.cc:89: last_sample_time_(base::TimeTicks()), nit: these initializers are probably not necessary, since it will use the default constructor for TimeTicks/TimeDelta by default without an initializer anyway
https://codereview.chromium.org/1776353006/diff/110001/chromecast/media/cma/p... File chromecast/media/cma/pipeline/media_pipeline_impl.cc (right): https://codereview.chromium.org/1776353006/diff/110001/chromecast/media/cma/p... chromecast/media/cma/pipeline/media_pipeline_impl.cc:61: if (estimated_bitrate_in_kbps <= 0) { On 2016/03/11 21:07:26, kmackay wrote: > nit: braces not necessary Done. https://codereview.chromium.org/1776353006/diff/110001/chromecast/media/cma/p... chromecast/media/cma/pipeline/media_pipeline_impl.cc:89: last_sample_time_(base::TimeTicks()), On 2016/03/11 21:07:26, kmackay wrote: > nit: these initializers are probably not necessary, since it will use the > default constructor for TimeTicks/TimeDelta by default without an initializer > anyway Done.
Spotted a crash bug - but looks good otherwise :) https://codereview.chromium.org/1776353006/diff/130001/chromecast/media/cma/p... File chromecast/media/cma/pipeline/media_pipeline_impl.cc (right): https://codereview.chromium.org/1776353006/diff/130001/chromecast/media/cma/p... chromecast/media/cma/pipeline/media_pipeline_impl.cc:443: audio_pipeline_->bytes_decoded_since_last_update(); It's possible to not have an audio pipeline, so need to guard with if (audio_pipeline_) https://codereview.chromium.org/1776353006/diff/130001/chromecast/media/cma/p... chromecast/media/cma/pipeline/media_pipeline_impl.cc:445: video_pipeline_->bytes_decoded_since_last_update(); Ditto.
https://codereview.chromium.org/1776353006/diff/130001/chromecast/media/cma/p... File chromecast/media/cma/pipeline/media_pipeline_impl.cc (right): https://codereview.chromium.org/1776353006/diff/130001/chromecast/media/cma/p... chromecast/media/cma/pipeline/media_pipeline_impl.cc:443: audio_pipeline_->bytes_decoded_since_last_update(); On 2016/03/11 21:22:36, halliwell wrote: > It's possible to not have an audio pipeline, so need to guard with if > (audio_pipeline_) Good catch, oversight on my part. Done. https://codereview.chromium.org/1776353006/diff/130001/chromecast/media/cma/p... chromecast/media/cma/pipeline/media_pipeline_impl.cc:445: video_pipeline_->bytes_decoded_since_last_update(); On 2016/03/11 21:22:36, halliwell wrote: > Ditto. Done.
On 2016/03/11 22:14:46, ejason wrote: > https://codereview.chromium.org/1776353006/diff/130001/chromecast/media/cma/p... > File chromecast/media/cma/pipeline/media_pipeline_impl.cc (right): > > https://codereview.chromium.org/1776353006/diff/130001/chromecast/media/cma/p... > chromecast/media/cma/pipeline/media_pipeline_impl.cc:443: > audio_pipeline_->bytes_decoded_since_last_update(); > On 2016/03/11 21:22:36, halliwell wrote: > > It's possible to not have an audio pipeline, so need to guard with if > > (audio_pipeline_) > > Good catch, oversight on my part. Done. > > https://codereview.chromium.org/1776353006/diff/130001/chromecast/media/cma/p... > chromecast/media/cma/pipeline/media_pipeline_impl.cc:445: > video_pipeline_->bytes_decoded_since_last_update(); > On 2016/03/11 21:22:36, halliwell wrote: > > Ditto. > > Done. lgtm
The CQ bit was checked by jasonroberts@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from kmackay@chromium.org Link to the patchset: https://codereview.chromium.org/1776353006/#ps150001 (title: "Handle different audio/video pipeline configurations")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1776353006/150001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1776353006/150001
Message was sent while issue was closed.
Description was changed from ========== [Chromecast] Add metrics for logging platform A/V bitrate estimates. BUG=internal b/26496758 ========== to ========== [Chromecast] Add metrics for logging platform A/V bitrate estimates. BUG=internal b/26496758 ==========
Message was sent while issue was closed.
Committed patchset #9 (id:150001)
Message was sent while issue was closed.
Description was changed from ========== [Chromecast] Add metrics for logging platform A/V bitrate estimates. BUG=internal b/26496758 ========== to ========== [Chromecast] Add metrics for logging platform A/V bitrate estimates. BUG=internal b/26496758 Committed: https://crrev.com/24fbb39886e90b81175b7fc9f8fc79677a53d176 Cr-Commit-Position: refs/heads/master@{#380787} ==========
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/24fbb39886e90b81175b7fc9f8fc79677a53d176 Cr-Commit-Position: refs/heads/master@{#380787} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
