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

Issue 272873002: Cast: Fix encoding event subscriber log packet size. (Closed)

Created:
6 years, 7 months ago by imcheng
Modified:
6 years, 7 months ago
Reviewers:
Alpha Left Google
CC:
chromium-reviews, hclam+watch_chromium.org, imcheng+watch_chromium.org, hguihot+watch_chromium.org, jasonroberts+watch_google.com, avayvod+watch_chromium.org, pwestin+watch_google.com, feature-media-reviews_chromium.org, jshin+watch_chromium.org, miu+watch_chromium.org, hubbe+watch_chromium.org, mikhal+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Cast: Fix encoding event subscriber log packet size. Previously we set the packet size in BasePacketEvent proto during construction and never overwrite it. The problem is that the proto is most often created as a result of seeing a receiver packet event first, which does not contain packet size information and thus the size field is set to 0. We'd need to overwrite the size field when we see a sender packet event, which contains packet size information. Most of the time we see receiver event first because they often arrive sooner (via RTCP packets) than sender events (via IPC every second). BUG=373418 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269513

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -0 lines) Patch
M media/cast/logging/encoding_event_subscriber.cc View 1 chunk +7 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
imcheng
This should fix the issue where we see very low values of transmission bitrate in ...
6 years, 7 months ago (2014-05-09 01:41:49 UTC) #1
Alpha Left Google
lgtm
6 years, 7 months ago (2014-05-09 01:55:08 UTC) #2
imcheng
The CQ bit was checked by imcheng@chromium.org
6 years, 7 months ago (2014-05-09 02:21:04 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/imcheng@chromium.org/272873002/1
6 years, 7 months ago (2014-05-09 02:27:40 UTC) #4
commit-bot: I haz the power
6 years, 7 months ago (2014-05-10 03:36:59 UTC) #5
Message was sent while issue was closed.
Change committed as 269513

Powered by Google App Engine
This is Rietveld 408576698