First page-set for media-related metrics measurement.

First page-set for media-related metrics measurement.




Create a page-set using which we can start obtaining media related 
performance metrics.

BUG=261318
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=212212

[anandc, 2013-07-17 21:01:19]

Hi Telemetry devs,

Now that the first media-related Telemetry Measurement has landed
(https://chromiumcodereview.appspot.com/16854013/), we are ready to start
creating page-sets to obtain real numbers.

PTAL.

Thanks.

[dtu, 2013-07-17 21:09:38]

https://codereview.chromium.org/19623003/diff/2001/tools/perf/page_sets/tough...
File tools/perf/page_sets/tough_video_cases.json (right):

https://codereview.chromium.org/19623003/diff/2001/tools/perf/page_sets/tough...
tools/perf/page_sets/tough_video_cases.json:1: {
You can also define actions at the top of the page set and refer to them. In
this case:

{
  "description": "lorem ipsum",
  "play_audio": [
    { "action: "play",
      "selector": "#audio_1",
      "wait_for_playing": true,
      "wait_for_ended": true
  ]
  "pages": [
    {
      "url":  "file://etaoinshrdlu.html",
      "media_metrics": { "action": "play_audio" }
  ]

[anandc, 2013-07-17 22:08:20]

Thanks, dtu@. PTAL.

+shadi@ FYI and any review comments.

https://codereview.chromium.org/19623003/diff/2001/tools/perf/page_sets/tough...
File tools/perf/page_sets/tough_video_cases.json (right):

https://codereview.chromium.org/19623003/diff/2001/tools/perf/page_sets/tough...
tools/perf/page_sets/tough_video_cases.json:1: {
On 2013/07/17 21:09:38, Dave Tu wrote:
> You can also define actions at the top of the page set and refer to them. In
> this case:
> 
> {
>   "description": "lorem ipsum",
>   "play_audio": [
>     { "action: "play",
>       "selector": "#audio_1",
>       "wait_for_playing": true,
>       "wait_for_ended": true
>   ]
>   "pages": [
>     {
>       "url":  "file://etaoinshrdlu.html",
>       "media_metrics": { "action": "play_audio" }
>   ]

Done.

[nduca, 2013-07-17 22:20:56]

i'm happy (lgtm) if dtu is happy. wait for his lg as well as mine.

[dtu, 2013-07-17 22:31:34]

lgtm

[commit-bot: I haz the power, 2013-07-17 22:46:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/anandc@chromium.org/19623003/7001

[scherkus (not reviewing), 2013-07-17 23:53:12]

On 2013/07/17 22:46:48, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> https://chromium-status.appspot.com/cq/anandc%40chromium.org/19623003/7001

does CQ support binary files? we may need to commit this manually...

Q for telemetry folks: are we happy with the name "tough_video_cases"? it looks
like the naming scheme was cribbed from the other folder names (e.g.,
tough_layer_cases), but I'd say most of these cases aren't "tough"

[anandc1, 2013-07-18 00:05:55]

This is the current CQ
status<https://chromium-status.appspot.com/cq/anandc%40chromium.org/19623003/7001>:
"Patch is ready to commit, but the CQ is delaying it because CQ has already
submitted 4 patches in the last 600 seconds"

When committing binary file was temporarily not supported, as happened
earlier this week, it says:
"

Binary file support is temporarilly disabled due to a bug. Please commit
blindly
the binary files first then commit the source change as a separate CL. Sorry
for
the annoyance.

"

The "tough*" naming was commented on during the original
CL<https://codereview.chromium.org/16854013/>
:

> Why did you guys go with tough_*_cases naming? Page sets don't have to be
>> "tough". Is it a strict naming convention? Why "tough"? I like the folder
>> &
>> .json separation though.
>>
>
> Its just what we do now. If you dont like it, you're welcome to put up a
> bulk
> rename to something better. I'd like to keep to the convention though, if
> thats
> okay with you.


Okay. A bulk rename is out of the scope of this CL anyway.






On Wed, Jul 17, 2013 at 4:53 PM, <scherkus@chromium.org> wrote:

> On 2013/07/17 22:46:48, I haz the power (commit-bot) wrote:
>
>> CQ is trying da patch. Follow status at
>> https://chromium-status.**appspot.com/cq/anandc%**
>>
40chromium.org/19623003/7001<https://chromium-status.appspot.com/cq/anandc%40chromium.org/19623003/7001>
>>
>
> does CQ support binary files? we may need to commit this manually...
>
> Q for telemetry folks: are we happy with the name "tough_video_cases"? it
> looks
> like the naming scheme was cribbed from the other folder names (e.g.,
> tough_layer_cases), but I'd say most of these cases aren't "tough"
>
>
https://chromiumcodereview.**appspot.com/19623003/<https://chromiumcodereview...
>

[commit-bot: I haz the power, 2013-07-18 00:42:00]

List of reviewers changed. anandc@google.com did a drive-by without LGTM'ing!

[commit-bot: I haz the power, 2013-07-18 02:08:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/anandc@chromium.org/19623003/7001

[commit-bot: I haz the power, 2013-07-18 02:20:36]

Change committed as 212212

[shadi, 2013-07-18 17:00:02]

Sorry for late review (I was out). We can deal with my comments in next CL
probably :).

Interesting that the media files landed, they must have fixed the binary files
in CQ issues. However, doing a sync I got most of them of size 0 bytes, maybe we
need to check them in again. 

There might be other issues with those content files that we will discuss
offline. 

Questions, should we put the media files in a media/ or content folder under the
video_cases folder?

https://chromiumcodereview.appspot.com/19623003/diff/7001/tools/perf/page_set...
File tools/perf/page_sets/tough_video_cases.json (right):

https://chromiumcodereview.appspot.com/19623003/diff/7001/tools/perf/page_set...
tools/perf/page_sets/tough_video_cases.json:15: }
Probably we can combine those two actions into one "play_media" actions.

https://chromiumcodereview.appspot.com/19623003/diff/7001/tools/perf/page_set...
tools/perf/page_sets/tough_video_cases.json:19: "url":
"file:///tough_video_cases/video.html?src=crowd.wav&id=audio_1&type=audio",
We don't really need to set the ID. We have two options: 
- Either play "selector": all.
- Preset media ID in the the html file. Since we know only one media is allowed
to play, then we can hard code its ID and use it in the play_media action.

https://chromiumcodereview.appspot.com/19623003/diff/7001/tools/perf/page_set...
File tools/perf/page_sets/tough_video_cases/video.html (right):

https://chromiumcodereview.appspot.com/19623003/diff/7001/tools/perf/page_set...
tools/perf/page_sets/tough_video_cases/video.html:27: qsParams =
getQueryStrings();
*var* qsParams

[anandc, 2013-08-01 18:34:19]

On 2013/07/18 17:00:02, shadi wrote:
> Sorry for late review (I was out). We can deal with my comments in next CL
> probably :).
> 
> Interesting that the media files landed, they must have fixed the binary files
> in CQ issues. However, doing a sync I got most of them of size 0 bytes, maybe
we
> need to check them in again. 
> 
> There might be other issues with those content files that we will discuss
> offline. 
> 
> Questions, should we put the media files in a media/ or content folder under
the
> video_cases folder?
> 
>
https://chromiumcodereview.appspot.com/19623003/diff/7001/tools/perf/page_set...
> File tools/perf/page_sets/tough_video_cases.json (right):
> 
>
https://chromiumcodereview.appspot.com/19623003/diff/7001/tools/perf/page_set...
> tools/perf/page_sets/tough_video_cases.json:15: }
> Probably we can combine those two actions into one "play_media" actions.
> 
>
https://chromiumcodereview.appspot.com/19623003/diff/7001/tools/perf/page_set...
> tools/perf/page_sets/tough_video_cases.json:19: "url":
> "file:///tough_video_cases/video.html?src=crowd.wav&id=audio_1&type=audio",
> We don't really need to set the ID. We have two options: 
> - Either play "selector": all.
> - Preset media ID in the the html file. Since we know only one media is
allowed
> to play, then we can hard code its ID and use it in the play_media action.
> 
>
https://chromiumcodereview.appspot.com/19623003/diff/7001/tools/perf/page_set...
> File tools/perf/page_sets/tough_video_cases/video.html (right):
> 
>
https://chromiumcodereview.appspot.com/19623003/diff/7001/tools/perf/page_set...
> tools/perf/page_sets/tough_video_cases/video.html:27: qsParams =
> getQueryStrings();
> *var* qsParams

The problem with binary files has been fixed by using Google Cloud Storage:
https://codereview.chromium.org/21053008/