Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(259)

Issue 2079923002: Combine bytes-decoded notification with memory usage and frame count. (Closed)

Created:
4 years, 6 months ago by alokp
Modified:
4 years, 5 months ago
CC:
chromium-reviews, feature-media-reviews_chromium.org, posciak+watch_chromium.org, xhwang
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Combine bytes-decoded notification with memory usage and frame count. This patch attempts to reduce the number of notifications being posted to the main thread and causing performance issues. To further reduce the number of notification we could consider combining AudioRendererImpl and VideoRendererImpl notifications in RendererImpl before sending it to PipelineImpl. BUG=619975

Patch Set 1 #

Patch Set 2 : rebase #

Total comments: 2

Patch Set 3 : rebase only #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -28 lines) Patch
M media/filters/decoder_stream.h View 1 chunk +3 lines, -0 lines 0 comments Download
M media/filters/decoder_stream.cc View 1 chunk +1 line, -1 line 0 comments Download
M media/filters/decoder_stream_traits.h View 2 chunks +0 lines, -4 lines 0 comments Download
M media/filters/decoder_stream_traits.cc View 2 chunks +0 lines, -16 lines 0 comments Download
M media/filters/video_frame_stream_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M media/renderers/audio_renderer_impl.h View 1 2 chunks +6 lines, -0 lines 0 comments Download
M media/renderers/audio_renderer_impl.cc View 1 4 chunks +12 lines, -2 lines 0 comments Download
M media/renderers/video_renderer_impl.h View 2 chunks +5 lines, -1 line 0 comments Download
M media/renderers/video_renderer_impl.cc View 5 chunks +11 lines, -2 lines 0 comments Download

Messages

Total messages: 11 (2 generated)
alokp
Perf tryjob is here: https://codereview.chromium.org/2075293003/
4 years, 6 months ago (2016-06-17 22:43:23 UTC) #2
DaleCurtis
https://codereview.chromium.org/2079923002/diff/20001/media/renderers/video_renderer_impl.cc File media/renderers/video_renderer_impl.cc (right): https://codereview.chromium.org/2079923002/diff/20001/media/renderers/video_renderer_impl.cc#newcode162 media/renderers/video_renderer_impl.cc:162: base::Bind(&VideoRendererImpl::BytesDecoded, weak_factory_.GetWeakPtr()), This is only returned when a frame ...
4 years, 6 months ago (2016-06-17 23:03:17 UTC) #4
alokp
https://codereview.chromium.org/2079923002/diff/20001/media/renderers/video_renderer_impl.cc File media/renderers/video_renderer_impl.cc (right): https://codereview.chromium.org/2079923002/diff/20001/media/renderers/video_renderer_impl.cc#newcode162 media/renderers/video_renderer_impl.cc:162: base::Bind(&VideoRendererImpl::BytesDecoded, weak_factory_.GetWeakPtr()), On 2016/06/17 23:03:17, DaleCurtis wrote: > This ...
4 years, 6 months ago (2016-06-18 04:18:06 UTC) #5
alokp
New tryrun because I forgot to specify pageset-repeat in the last run: https://codereview.chromium.org/2080483002/
4 years, 6 months ago (2016-06-18 04:19:25 UTC) #6
DaleCurtis
Results: http://storage.googleapis.com/chromium-telemetry/html-results/results-2016-06-17_23-26-39 -- I forget what the original values were but it still looks like ...
4 years, 6 months ago (2016-06-20 18:45:39 UTC) #7
alokp
On 2016/06/20 18:45:39, DaleCurtis wrote: > Results: > http://storage.googleapis.com/chromium-telemetry/html-results/results-2016-06-17_23-26-39 > -- I forget what the ...
4 years, 6 months ago (2016-06-21 22:12:21 UTC) #8
DaleCurtis
On 2016/06/21 at 22:12:21, alokp wrote: > On 2016/06/20 18:45:39, DaleCurtis wrote: > > Results: ...
4 years, 6 months ago (2016-06-21 23:50:03 UTC) #9
alokp
> Can we remove the posting entirely and just update local variables that can be ...
4 years, 6 months ago (2016-06-22 02:35:04 UTC) #10
DaleCurtis
4 years, 6 months ago (2016-06-22 15:12:34 UTC) #11
Yeah, I think that sounds good and is more efficient.

Powered by Google App Engine
This is Rietveld 408576698