Issue metadata
Sign in to add a comment
|
7.4%-237.7% regression in webrtc_perf_tests at 16705:16705 |
||||||||||||||||||
Issue descriptionSee graphs below.
,
Feb 23 2017
Hi Tommi, It looks like your CL (https://codereview.webrtc.org/2708433002) is causing some perf regressions (and a few improvements). Could you have a look if that's expected?
,
Feb 23 2017
We were hoping for improvements only :) Stefan - can you take a look at this with me?
,
Feb 23 2017
I think it looks worrying that encode and decode time is twice as long on average on Nexus 9. It doesn't look like the test tries to poll stats too often: https://cs.chromium.org/chromium/src/third_party/webrtc/video/video_quality_test.cc?rcl=cc7a3f62e4339daa1021b1bad24507aea24942ef&l=598 It could be that the fake network pipe is affecting performance, but it should also only run with a 5 ms interval: https://cs.chromium.org/chromium/src/third_party/webrtc/test/fake_network_pipe.cc?rcl=cc7a3f62e4339daa1021b1bad24507aea24942ef&l=208
,
Feb 23 2017
Oh, so nanosleep() doesn't yield?
,
Feb 23 2017
Is the 'compare' thread perhaps running in a tight loop without making much progress? We could swap out the nanosleep for sched_yield. When we did add nanosleep though, the linux_memcheck bot got responsive :) whereas it wasn't before we did so.
,
Feb 23 2017
I landed a little change in webrtc that swaps out nanosleep for sched_yield. It's being rolled into chrome at the moment.
,
Feb 24 2017
That change doesn't seem to have made a noticeable difference.
,
Feb 24 2017
I added about 60 new alerts that were triggered by the "little change" CL (https://codereview.webrtc.org/2716683002). Most seem to be regressions, but there are some improvements as well.
,
Feb 24 2017
hah - thanks Ivo. I've reverted that 'little' change now. As for the improvements/regressions, they seem to be tied to specific platforms: Regression: webrtc-android-tests-nexus9 Improvement: webrtc-linux-large-tests webrtc-mac-large-tests
,
Feb 24 2017
Are all Android bots running the same OS version?
,
Feb 24 2017
Good question. I know they don't all run the same architecture at least (Nexus 9 runs on NVIDIA Tegra K1 fwict)
,
Feb 24 2017
#9 - When I look at https://chromeperf.appspot.com/group_report?bug_id=695438, there seems to be mostly improvements (some very slight though). E.g. https://chromeperf.appspot.com/report?sid=f66f458698d9aacf9a55d79594aa72bfb4df93fc1fb3519819d75ccf2093d138&rev=16807 and https://chromeperf.appspot.com/report?sid=5c3cd95612aa837642e5ffea9466b2a6965ed64ac2488b29f89e6b75855677af&rev=16807 I must be reading the graphs wrong. Please explain!
,
Feb 24 2017
I'm not fully aware of the mechanics in this test. I would assume that we don't have more than 4 cores on this device, so we're running a single frame comparison thread. It doesn't look like it would spin. The decode thread is set to kHighPriority. Can we see exactly what system priorities threads run at? If, say, max/min prio fetched from the OS are too close, we'd end up with Normal/High being assigned the same system priority. Seems unlikely though.
,
Feb 24 2017
wow... I must be reading it wrong too. Magnus, Stefan - can you take a look at the decoding and encoding changes? Both the initial perf changes as reported at the top (improvements on linux, mac, regression on nexus 9) and the charts that Fredrik points to in comment #13.
,
Feb 25 2017
I'm tempted to reland the 'little' change actually and start tracking down the places where I suspect that we have tight noop loops that eat up the CPU or otherwise don't play nice with some schedulers. looking at all the alerts: https://chromeperf.appspot.com/group_report?rev=16807 I'm in the process of removing dependency on the loop in PlatformThread, which forces us to examine all uses of that loop and re-evaluate what sort of loop is appropriate in each case (and if TaskQueue might be more appropriate in some cases).
,
Feb 26 2017
I found and removed one busy loop in our test code and the perf bots are catching up. Here's the CL: https://chromium.googlesource.com/external/webrtc/+/4af9f962c3a79b97c4b2d85be4c30c9b77ef7a21 Here's the first alert I've seen: https://chromeperf.appspot.com/report?sid=1a61fdf8a97c6612d63a8f5248a7c5f94e65eb3d1192d67bef2c4793fd427f15&rev=16837 I suspect we might have more that could be skewing perf measurements (i.e. a more efficient loop implementation will cause a busy loop to use more CPU -> performance regression).
,
Feb 27 2017
Found and fixed another issue when the NackModule is disabled: https://bugs.chromium.org/p/webrtc/issues/detail?id=7246
,
Feb 27 2017
Found one more, but this one requires someone familiar with the pacer code to take a look: https://bugs.chromium.org/p/webrtc/issues/detail?id=7255
,
Feb 27 2017
FakeNetworkPipe is another class that causes a tight loop.
,
Feb 28 2017
Most of the loop issues have now been fixed and I've reverted back to using only sched_yield and not nanosleep in PlatformThread. Philip is looking at the PacerThread issue in this bug: https://bugs.chromium.org/p/webrtc/issues/detail?id=7255
,
Feb 28 2017
sched_yield change got reverted again. Philip found a busy loop in the video decoder and fixed it (now in cq). Fix for the pacer is also in the cq.
,
Feb 28 2017
nice :)
,
Feb 28 2017
wow!
,
Mar 4 2017
The Nexus9 perf numbers still don't make sense to me. While all other platforms see improvements, Nexus9 shows regressions across all multiple "foreman_cif_*" tests. Is there a chance that these numbers are somehow incorrectly calculated in those tests on those bots (or that bot?)? Or is there a chance that with the improvements, the tests are somehow doing more, which results in these specific numbers regressing? (e.g. as in "decoding more frames will consume more cpu") Stefan - I'm assigning this to you for now to take a look. As far as the other numbers, I think we're happy and won't analyze further.
,
Mar 10 2017
tommi, I'm not sure what I should take a look at. It's pretty clear from the graphs that it now takes a longer time to decode frames on some nexus devices. decode_time / foreman_cif_1000kbps_100ms_32pkts_queue has gone from 1.5 ms to 3.5 ms on average. This is measured as the time between passing a frame to decoder and getting it back out again. I don't see how it should matter if we're able to do more or not? Maybe if we're doing something wrong in how we compute an average on Android, but I think that's pretty far fetched. :) Could it be that we're somehow having more context switches?
,
Mar 10 2017
Is Nexus9 running a different scheduler than the other android devices? As far as the loop goes, which is what triggered the change, I don't think it should increase the number of context switches, reduce if anything, but do you know how that could affect the decoder? Actually, can you point me to the code where we pass the frame to the decoder and where the place is where we consider the decoding done? I'm assuming those places are not synchronous but are somehow affected by using an atomic in the thread loop instead of a mutex.
,
Mar 10 2017
Sure, here it is: https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/generic_decoder.cc?rcl=13fe480c34baeafe3575ca515a23112c680c141b&l=168 https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/generic_decoder.cc?rcl=13fe480c34baeafe3575ca515a23112c680c141b&l=82 The nexus 9 has two cores and the decoder always uses a single thread if this is running with libvpx.
,
Mar 10 2017
Thanks for the info. From what I can tell now, it appears that threading on Android is different than on the other platforms. On all platforms where there was an improvement in the measurement, decoding happens on the same thread as the timing measurements are made. On Android, it looks like the first timestamp is captured on the decoding thread, decoding is done and then the 'done' timestamp is captured on a different thread at some later time. The measurement doesn't reflect the actual decoding operation but rather shows that the frame notification arrives later on a different thread, possibly because we're using the decoder thread more efficiently (and not actually yielding to the other thread as frequently as before). Here's my test CL: https://codereview.webrtc.org/2744813002/ I'm adding thread checks etc there now and establishing what the threading model is in generic decoder classes. It looks like there are opportunities there to reduce locking. I also think it would be good to have the same threading expectations across all platforms.
,
Mar 12 2017
Some more updates. Indeed the decoder on Android, MediaCodecVideoDecoder, delivers frames back on a different thread than the decoder thread. This is part of what's behind the perf regression there. #1. We need to fix it so that Decoded() callbacks consistently happen on the same thread (or queue) across all platforms. #2. However, as it stands, that's not a simple thing to do because the decoder thread is a PlatformThread that just runs a decode loop+sleep, with no way of posting tasks to it. It would be good to use a TaskQueue there and from what I can tell, it could simplify things a *lot* by tying usage of some objects to that queue. As is, we incorporate a lot of locking, sleeping/waiting with events, which triggers a lot of context switches (besides just blocking threads). #3. Each decoder uses at least one thread (2 or more on Android). By using a TaskQueue, we could use a single task queue for multiple decoders, which could also be a way to apply CPU adaptation to the video streams. (E.g. calculate how many can share a task queue, possibly create more task queues as needed, or have a deterministic roof). Doing the above is a fairly major refactoring, but there are steps to get there and I've got a hack that I'm trying out now, that partially moves the poll for decoded threads in MediaCodecVideoDecoder over to the decoder thread, to at least get us to uniform behavior. One interesting thing I noticed too, is that if we do not do a nanosleep() in the decoder thread loop (which loops and sleeps anyway), the Pacer and Voice threads, start tripping the "busy loop" checking in PlatformThread. It could be that CPU frequency scaling is a factor there as well and removing the nanosleep, causes the frequency to go up. That may also have been a factor when considering the regression. If so, then it would seem that while the time it takes for a decoded frame to be decoded and delivered on Android, increased because CPU frequency went down - not because we're actually using more CPU.
,
Mar 12 2017
Some background info on MediaCodecVideoDecoder/androidmediadecoder_jni in WebRTC; we discussed if we should refactor it in Q4 last year, to reduce the number of threads/polling, and clean it up in general. We had two different possibilities: 1. There is a new MediaCodec interface available on Lollipop and above that allows us to run it in asynchronous mode, i.e. we get notified with a callback when a frame is decoded instead of us having to poll it all the time. Using the asynchronous mode, we would get rid of the polling, and possible the use of |codec_thread_| in androidmediadecoder_jni altogether. 2. MediaCodec is available in the NDK, i.e. we could do a pure C++ implementation and skip all the JNI communication, which would simplify the code a lot. It would be optimal if we could do both, i.e. do it purely in C++ AND remove the polling, but unfortunately, the NDK does not expose the asynchronous mode for some reason. We filed a bug to expose the asynchronous mode in the NDK interface in b/31792901, and put the refactoring on ice, since we couldn't get both benefits. The plan was also to not touch the existing decoder, but make a complete rewrite and introduce it as a new webrtc::VideoDecoder alongside the old decoder, so we could roll it out incrementally. The MediaCodecVideoDecoder is messy and brittle, and making changes to it in the past has caused trouble for the clients using it.
,
Mar 13 2017
Stefan - I'll take this bug off your back again. Thanks for the pointers for where the measurements are. I understand the flow better now, am preparing a few CLs(as you've seen) in the area and am hopeful we we won't have to just accept these regressions as they are. Magnus - thanks for the background on the Android media codec. Very useful and good to know we've been thinking about this and that there might be options for doing things differently. It does sound like the refactoring work will take some time and I'd like to have a way to get the decoder threading in general (as in generic_decoder.*) be consistent across platforms. Once we're there, we can update the Android implementation and improve the decoding framework in parallel. I'll send a proposal your way for an intermediate step.
,
Mar 13 2017
,
Feb 21 2018
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/a4e71b9e7e59be21b98d63cf8cb676096d9c74b0 commit a4e71b9e7e59be21b98d63cf8cb676096d9c74b0 Author: Tommi <tommi@webrtc.org> Date: Wed Feb 21 09:27:06 2018 VCMGenericDecoder threading updates for all but Android. * All methods now have thread checks. * Variable access associated with thread checkers. * Remove need for |rtc::CriticalSection lock_| Since the android decoder is inherently asynchronous, and FrameBuffer2's decoder doesn't support posting tasks to it yet (for async decode completion), we need to tackle android separately. Once FrameBuffer2 gets changed to use a TaskQueue or ProcessThread, we can move Android over to delivering decoded frames on the right thread/queue and delete generic_decoder_android.*. Note: This is a subset of code that was previously reviewed here: - https://codereview.webrtc.org/2764573002/ Bug: webrtc:7361, webrtc:8907, chromium:695438 Change-Id: I118609dfa5c0f0180287d8c2b6d62987b7473c5c Reviewed-on: https://webrtc-review.googlesource.com/55060 Commit-Queue: Tommi <tommi@webrtc.org> Reviewed-by: Sami Kalliomäki <sakal@webrtc.org> Cr-Commit-Position: refs/heads/master@{#22119} [modify] https://crrev.com/a4e71b9e7e59be21b98d63cf8cb676096d9c74b0/modules/video_coding/BUILD.gn [modify] https://crrev.com/a4e71b9e7e59be21b98d63cf8cb676096d9c74b0/modules/video_coding/generic_decoder.cc [modify] https://crrev.com/a4e71b9e7e59be21b98d63cf8cb676096d9c74b0/modules/video_coding/generic_decoder.h [add] https://crrev.com/a4e71b9e7e59be21b98d63cf8cb676096d9c74b0/modules/video_coding/generic_decoder_android.cc [add] https://crrev.com/a4e71b9e7e59be21b98d63cf8cb676096d9c74b0/modules/video_coding/generic_decoder_android.h
,
Feb 21 2018
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/9f016a0eb01db60c55dad640ddc03562d88cc087 commit 9f016a0eb01db60c55dad640ddc03562d88cc087 Author: Tommi <tommi@webrtc.org> Date: Wed Feb 21 11:25:02 2018 Comment out DCHECK in dtor of VCMDecodedFrameCallback. Looking into the downstream issue now. NoTry: true Tbr: ossu@webrtc.org Bug: webrtc:7361, webrtc:8907, chromium:695438 Change-Id: Ib52b86cf26401c490b415b151916ec35f0716345 Reviewed-on: https://webrtc-review.googlesource.com/56042 Reviewed-by: Tommi <tommi@webrtc.org> Commit-Queue: Tommi <tommi@webrtc.org> Cr-Commit-Position: refs/heads/master@{#22122} [modify] https://crrev.com/9f016a0eb01db60c55dad640ddc03562d88cc087/modules/video_coding/generic_decoder.cc
,
Feb 21 2018
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/c75f1e45093a8d5cc62937c7708b87aa5c5bf0b0 commit c75f1e45093a8d5cc62937c7708b87aa5c5bf0b0 Author: Tommi <tommi@webrtc.org> Date: Wed Feb 21 15:44:05 2018 Reduce locking in VideoReceiver and check the threading model. Note: This is a subset of code that was previously reviewed here: - https://codereview.webrtc.org/2764573002/ * Added two notification methods, DecoderThreadStarting() and DecoderThreadStopped() * Allows us to establish a period when the decoder thread is not running and it is safe to modify variables such as callbacks, that are only read when the decoder thread is running. * Allows us to DCHECK thread guarantees/correctness. * Allows synchronizing callbacks from the module process thread and have them only active while the decoder thread is running. * The above, allows us to establish two modes for the thread, single-threaded-mutable and multi-threaded-const. * Using that knowledge, we can remove |receive_crit_| as well as locking for a number of member variables. * Removed |VCMFrameBuffer _frameFromFile| (unused). * Clean up several of my TODOs Bug: webrtc:7361, chromium:695438 Change-Id: Id0048ee9624f76103c088d02825eb5c0d6c8913c Reviewed-on: https://webrtc-review.googlesource.com/55000 Commit-Queue: Tommi <tommi@webrtc.org> Reviewed-by: Philip Eliasson <philipel@webrtc.org> Cr-Commit-Position: refs/heads/master@{#22133} [modify] https://crrev.com/c75f1e45093a8d5cc62937c7708b87aa5c5bf0b0/modules/video_coding/video_coding_impl.h [modify] https://crrev.com/c75f1e45093a8d5cc62937c7708b87aa5c5bf0b0/modules/video_coding/video_receiver.cc [modify] https://crrev.com/c75f1e45093a8d5cc62937c7708b87aa5c5bf0b0/video/video_receive_stream.cc
,
Feb 21 2018
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/c4f9824cee43d32cb6296d7d58c8bfb8951f0f0e commit c4f9824cee43d32cb6296d7d58c8bfb8951f0f0e Author: Lu Liu <lliuu@webrtc.org> Date: Wed Feb 21 19:37:21 2018 Revert "Reduce locking in VideoReceiver and check the threading model." This reverts commit c75f1e45093a8d5cc62937c7708b87aa5c5bf0b0. Reason for revert: Breaking internal project Original change's description: > Reduce locking in VideoReceiver and check the threading model. > > Note: This is a subset of code that was previously reviewed here: > - https://codereview.webrtc.org/2764573002/ > > * Added two notification methods, DecoderThreadStarting() and DecoderThreadStopped() > * Allows us to establish a period when the decoder thread is not running and it is > safe to modify variables such as callbacks, that are only read when the decoder > thread is running. > * Allows us to DCHECK thread guarantees/correctness. > * Allows synchronizing callbacks from the module process thread and have them only > active while the decoder thread is running. > * The above, allows us to establish two modes for the thread, > single-threaded-mutable and multi-threaded-const. > * Using that knowledge, we can remove |receive_crit_| as well as locking for a > number of member variables. > * Removed |VCMFrameBuffer _frameFromFile| (unused). > * Clean up several of my TODOs > > Bug: webrtc:7361, chromium:695438 > Change-Id: Id0048ee9624f76103c088d02825eb5c0d6c8913c > Reviewed-on: https://webrtc-review.googlesource.com/55000 > Commit-Queue: Tommi <tommi@webrtc.org> > Reviewed-by: Philip Eliasson <philipel@webrtc.org> > Cr-Commit-Position: refs/heads/master@{#22133} TBR=tommi@webrtc.org,philipel@webrtc.org Change-Id: I4d78e8b2c05b36e1a3f64cb38d652579b3a23f22 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:7361, chromium:695438 Reviewed-on: https://webrtc-review.googlesource.com/56280 Reviewed-by: Lu Liu <lliuu@webrtc.org> Commit-Queue: Lu Liu <lliuu@webrtc.org> Cr-Commit-Position: refs/heads/master@{#22141} [modify] https://crrev.com/c4f9824cee43d32cb6296d7d58c8bfb8951f0f0e/modules/video_coding/video_coding_impl.h [modify] https://crrev.com/c4f9824cee43d32cb6296d7d58c8bfb8951f0f0e/modules/video_coding/video_receiver.cc [modify] https://crrev.com/c4f9824cee43d32cb6296d7d58c8bfb8951f0f0e/video/video_receive_stream.cc
,
Feb 21 2018
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/54daa3ac4df4f467f8cc28d54c21cbd3f8e58bbe commit 54daa3ac4df4f467f8cc28d54c21cbd3f8e58bbe Author: Lu Liu <lliuu@webrtc.org> Date: Wed Feb 21 19:38:24 2018 Revert "Comment out DCHECK in dtor of VCMDecodedFrameCallback." This reverts commit 9f016a0eb01db60c55dad640ddc03562d88cc087. Reason for revert: Breaking internal project Original change's description: > Comment out DCHECK in dtor of VCMDecodedFrameCallback. > Looking into the downstream issue now. > > NoTry: true > Tbr: ossu@webrtc.org > Bug: webrtc:7361, webrtc:8907, chromium:695438 > Change-Id: Ib52b86cf26401c490b415b151916ec35f0716345 > Reviewed-on: https://webrtc-review.googlesource.com/56042 > Reviewed-by: Tommi <tommi@webrtc.org> > Commit-Queue: Tommi <tommi@webrtc.org> > Cr-Commit-Position: refs/heads/master@{#22122} TBR=ossu@webrtc.org,tommi@webrtc.org Change-Id: I096205c1fe70131f6e1c866411f8838e12eafa92 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:7361, webrtc:8907, chromium:695438 Reviewed-on: https://webrtc-review.googlesource.com/56281 Reviewed-by: Lu Liu <lliuu@webrtc.org> Commit-Queue: Lu Liu <lliuu@webrtc.org> Cr-Commit-Position: refs/heads/master@{#22142} [modify] https://crrev.com/54daa3ac4df4f467f8cc28d54c21cbd3f8e58bbe/modules/video_coding/generic_decoder.cc
,
Feb 21 2018
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/352314adb8f4d59e15e7344ee0aec3abcf3f446e commit 352314adb8f4d59e15e7344ee0aec3abcf3f446e Author: Lu Liu <lliuu@webrtc.org> Date: Wed Feb 21 19:39:29 2018 Revert "VCMGenericDecoder threading updates for all but Android." This reverts commit a4e71b9e7e59be21b98d63cf8cb676096d9c74b0. Reason for revert: Breaking internal project Original change's description: > VCMGenericDecoder threading updates for all but Android. > > * All methods now have thread checks. > * Variable access associated with thread checkers. > * Remove need for |rtc::CriticalSection lock_| > > Since the android decoder is inherently asynchronous, and > FrameBuffer2's decoder doesn't support posting tasks to it > yet (for async decode completion), we need to tackle android > separately. Once FrameBuffer2 gets changed to use a TaskQueue > or ProcessThread, we can move Android over to delivering decoded > frames on the right thread/queue and delete generic_decoder_android.*. > > Note: This is a subset of code that was previously reviewed here: > - https://codereview.webrtc.org/2764573002/ > > Bug: webrtc:7361, webrtc:8907, chromium:695438 > Change-Id: I118609dfa5c0f0180287d8c2b6d62987b7473c5c > Reviewed-on: https://webrtc-review.googlesource.com/55060 > Commit-Queue: Tommi <tommi@webrtc.org> > Reviewed-by: Sami Kalliomäki <sakal@webrtc.org> > Cr-Commit-Position: refs/heads/master@{#22119} TBR=sakal@webrtc.org,tommi@webrtc.org Change-Id: I3afe4671f9d06bb4a2b17e4f14c21d79f773e067 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:7361, webrtc:8907, chromium:695438 Reviewed-on: https://webrtc-review.googlesource.com/56282 Reviewed-by: Lu Liu <lliuu@webrtc.org> Commit-Queue: Lu Liu <lliuu@webrtc.org> Cr-Commit-Position: refs/heads/master@{#22143} [modify] https://crrev.com/352314adb8f4d59e15e7344ee0aec3abcf3f446e/modules/video_coding/BUILD.gn [modify] https://crrev.com/352314adb8f4d59e15e7344ee0aec3abcf3f446e/modules/video_coding/generic_decoder.cc [modify] https://crrev.com/352314adb8f4d59e15e7344ee0aec3abcf3f446e/modules/video_coding/generic_decoder.h [delete] https://crrev.com/54daa3ac4df4f467f8cc28d54c21cbd3f8e58bbe/modules/video_coding/generic_decoder_android.cc [delete] https://crrev.com/54daa3ac4df4f467f8cc28d54c21cbd3f8e58bbe/modules/video_coding/generic_decoder_android.h
,
Feb 21 2018
so close...
,
Feb 22 2018
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/fbf3bce431000fcc315d91602c8d8660ed03dae3 commit fbf3bce431000fcc315d91602c8d8660ed03dae3 Author: Tommi <tommi@webrtc.org> Date: Thu Feb 22 18:03:45 2018 Reland "Reduce locking in VideoReceiver and check the threading model." This is a reland of c75f1e45093a8d5cc62937c7708b87aa5c5bf0b0. Original change's description: > Reduce locking in VideoReceiver and check the threading model. > > Note: This is a subset of code that was previously reviewed here: > - https://codereview.webrtc.org/2764573002/ > > * Added two notification methods, DecoderThreadStarting() and DecoderThreadStopped() > * Allows us to establish a period when the decoder thread is not running and it is > safe to modify variables such as callbacks, that are only read when the decoder > thread is running. > * Allows us to DCHECK thread guarantees/correctness. > * Allows synchronizing callbacks from the module process thread and have them only > active while the decoder thread is running. > * The above, allows us to establish two modes for the thread, > single-threaded-mutable and multi-threaded-const. > * Using that knowledge, we can remove |receive_crit_| as well as locking for a > number of member variables. > * Removed |VCMFrameBuffer _frameFromFile| (unused). > * Clean up several of my TODOs > > Bug: webrtc:7361, chromium:695438 > Change-Id: Id0048ee9624f76103c088d02825eb5c0d6c8913c > Reviewed-on: https://webrtc-review.googlesource.com/55000 > Commit-Queue: Tommi <tommi@webrtc.org> > Reviewed-by: Philip Eliasson <philipel@webrtc.org> > Cr-Commit-Position: refs/heads/master@{#22133} Bug: webrtc:7361, chromium:695438 Change-Id: I32e1dc6c62cb30ad96e6366106f39fe415de49f1 Tbr: philipel@webrtc.org Reviewed-on: https://webrtc-review.googlesource.com/56803 Commit-Queue: Tommi <tommi@webrtc.org> Reviewed-by: Tommi <tommi@webrtc.org> Cr-Commit-Position: refs/heads/master@{#22163} [modify] https://crrev.com/fbf3bce431000fcc315d91602c8d8660ed03dae3/modules/video_coding/video_coding_impl.h [modify] https://crrev.com/fbf3bce431000fcc315d91602c8d8660ed03dae3/modules/video_coding/video_receiver.cc [modify] https://crrev.com/fbf3bce431000fcc315d91602c8d8660ed03dae3/video/video_receive_stream.cc |
|||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||
Comment 1 by ivoc@chromium.org
, Feb 23 2017