New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 711792 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Watch time reporting does not support adaptations from < 200p to > 200p.

Project Member Reported by dalecur...@chromium.org, Apr 14 2017

Issue description

Since 140p is a YouTube format this is probably missing a fair bit of watch time.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2540556a43a467a11d4c2cffdb9457b97d8b94c4

commit 2540556a43a467a11d4c2cffdb9457b97d8b94c4
Author: dalecurtis <dalecurtis@chromium.org>
Date: Fri Apr 14 23:35:11 2017

Start reporting watch time if ABR adapts above 200p.

Previously we only checked the initial resolution and assumed it
was always large enough to start watch time reporting; clearly
this is not true since YouTube offers a 140p format.

Probably we also want to drop the minimum resolution to 200x140p,
which I'll check with product about and handle in a followup CL.

BUG= 711792 
TEST=new tests. Manual: force 140p, adapt above, verify watch time.

Review-Url: https://codereview.chromium.org/2822543006
Cr-Commit-Position: refs/heads/master@{#464826}

[modify] https://crrev.com/2540556a43a467a11d4c2cffdb9457b97d8b94c4/media/blink/watch_time_reporter.cc
[modify] https://crrev.com/2540556a43a467a11d4c2cffdb9457b97d8b94c4/media/blink/watch_time_reporter.h
[modify] https://crrev.com/2540556a43a467a11d4c2cffdb9457b97d8b94c4/media/blink/watch_time_reporter_unittest.cc
[modify] https://crrev.com/2540556a43a467a11d4c2cffdb9457b97d8b94c4/media/blink/webmediaplayer_impl.cc

Labels: Merge-Request-59
Labels: -OS-Linux OS-All
Project Member

Comment 4 by sheriffbot@chromium.org, Apr 17 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 17 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c8965aac50f0f4772195c2088404b1bc040dc5b

commit 4c8965aac50f0f4772195c2088404b1bc040dc5b
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Mon Apr 17 19:03:04 2017

Merge M59: "Start reporting watch time if ABR adapts above 200p."

Previously we only checked the initial resolution and assumed it
was always large enough to start watch time reporting; clearly
this is not true since YouTube offers a 140p format.

Probably we also want to drop the minimum resolution to 200x140p,
which I'll check with product about and handle in a followup CL.

BUG= 711792 
TEST=new tests. Manual: force 140p, adapt above, verify watch time.

Review-Url: https://codereview.chromium.org/2822543006
Cr-Commit-Position: refs/heads/master@{#464826}
(cherry picked from commit 2540556a43a467a11d4c2cffdb9457b97d8b94c4)

Review-Url: https://codereview.chromium.org/2819323002 .
Cr-Commit-Position: refs/branch-heads/3071@{#21}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/4c8965aac50f0f4772195c2088404b1bc040dc5b/media/blink/watch_time_reporter.cc
[modify] https://crrev.com/4c8965aac50f0f4772195c2088404b1bc040dc5b/media/blink/watch_time_reporter.h
[modify] https://crrev.com/4c8965aac50f0f4772195c2088404b1bc040dc5b/media/blink/watch_time_reporter_unittest.cc
[modify] https://crrev.com/4c8965aac50f0f4772195c2088404b1bc040dc5b/media/blink/webmediaplayer_impl.cc

Status: Fixed (was: Started)

Sign in to add a comment