[media telemetry] Don't edit commandline flag defaults to exclude stories.

Remove "options = {'story_tag_filter_exclude': 'is_4k,is_50fps'}"
This isn't really a good way to do this because of the following.

if you run the code like this:
$ ./tools/perf/run_benchmark run media.mobile --browser=android-chrome
then the filter will work:
   number of stories 22

but if you run the code like this then
$ ./tools/perf/run_benchmark run media.mobile --browser=android-chrome --story-tag-filter-exclude='other_tag'
then the filter will not work:
   number of stories 29 (which means the 4k stories and 50fps stories were not excluded.)
This is because the "options" simply sets the default "--story-tag-filter-exclude"
So when a user provides a different value, the default is overriden.

My change fixes this such that you can use that flag without side effects.
It also helps by making the other parts of the system be able to know
by default what stories are in the page_set. You can see that this is
true because the the autogenerated benchmark.csv file now has fewer
tags for media.mobile because of the filtered stories.


I am attempting to redesign this
system a bit for https://goo.gl/JZ2yZQ. I noticed this code.


Also remove
tag = 'android'
since it doesn't do anything.


Bug: 894254
Change-Id: Ibf5f90e83062c785934af334c8dd6bfde89cef25
Reviewed-on: https://chromium-review.googlesource.com/c/1356320
Commit-Queue: Caleb Rouleau <crouleau@chromium.org>
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612840}

tools/perf/benchmark.csv

@@ -27,7 +27,7 @@ load_library_perf_tests,"xhwang@chromium.org, crouleau@chromium.org",Internals>M
 loading.desktop,"kouhei@chromium.org, ksakamoto@chromium.org",,https://bit.ly/loading-benchmarks,"cache_temperature_cold,cache_temperature_warm,international,intl_ar_fa_he,intl_es_fr_pt_BR,intl_hi_ru,intl_ja_zh,intl_ko_th_vi,typical"
 loading.mobile,"kouhei@chromium.org, ksakamoto@chromium.org",,https://bit.ly/loading-benchmarks,"cache_temperature_cold,cache_temperature_hot,cache_temperature_warm,easy_ttfmp,easy_tti,global,pwa,tough_ttfmp,tough_tti"
 media.desktop,dalecurtis@chromium.org,Internals>Media,,"aac,audio_only,audio_video,background,beginning_to_end,busyjs,cns,h264,is_4k,is_50fps,mp3,mse,opus,pcm,seek,src,video_only,vorbis,vp8,vp9"
-media.mobile,dalecurtis@chromium.org,Internals>Media,,"aac,audio_only,audio_video,background,beginning_to_end,busyjs,cns,h264,is_4k,is_50fps,mp3,mse,opus,pcm,seek,src,video_only,vorbis,vp8,vp9"
+media.mobile,dalecurtis@chromium.org,Internals>Media,,"aac,audio_only,audio_video,background,beginning_to_end,busyjs,cns,h264,mp3,mse,opus,pcm,seek,src,video_only,vorbis,vp9"
 media_perftests,"crouleau@chromium.org, dalecurtis@chromium.org",Internals>Media,,
 memory.desktop,erikchen@chromium.org,,,
 memory.long_running_idle_gmail_background_tbmv2,ulan@chromium.org,,,

tools/perf/benchmarks/media.py

@@ -28,9 +28,6 @@ class _MediaBenchmark(perf_benchmark.PerfBenchmark):
   """Base class for TBMv2-based media benchmarks (MediaDesktop and
   MediaMobile)."""
 
-  def CreateStorySet(self, options):
-    return page_sets.MediaCasesStorySet(measure_memory=True)
-
   def CreateCoreTimelineBasedMeasurementOptions(self):
     category_filter = chrome_trace_category_filter.ChromeTraceCategoryFilter()
 
@@ -68,6 +65,9 @@ class MediaDesktop(_MediaBenchmark):
   """Obtains media performance for key user scenarios on desktop."""
   SUPPORTED_PLATFORMS = [story.expectations.ALL_DESKTOP]
 
+  def CreateStorySet(self, options):
+    return page_sets.MediaCasesDesktopStorySet(measure_memory=True)
+
   @classmethod
   def Name(cls):
     return 'media.desktop'
@@ -79,10 +79,11 @@ class MediaDesktop(_MediaBenchmark):
 class MediaMobile(_MediaBenchmark):
   """Obtains media performance for key user scenarios on mobile devices."""
 
-  tag = 'android'
-  options = {'story_tag_filter_exclude': 'is_4k,is_50fps'}
   SUPPORTED_PLATFORMS = [story.expectations.ANDROID_NOT_WEBVIEW]
 
+  def CreateStorySet(self, options):
+    return page_sets.MediaCasesMobileStorySet(measure_memory=True)
+
   @classmethod
   def Name(cls):
     return 'media.mobile'

tools/perf/page_sets/media_cases.py

@@ -7,6 +7,8 @@
 # Silverlight). These scenarios simply exercise the Chromium code in particular
 # ways. The metrics that are produced are calculated in a separate step.
 
+import abc
+
 from telemetry.page import page as page_module
 from telemetry.page import traffic_setting as traffic_setting_module
 from telemetry import story
@@ -45,6 +47,7 @@ _PAGE_TAGS_LIST = [
     'mse'
 ]
 
+
 # A list of traffic setting names to append to page names when used.
 # Traffic settings is a way to constrain the network to match real-world
 # scenarios.
@@ -201,139 +204,164 @@ class _MSEPage(_MediaPage):
       raise RuntimeError(action_runner.EvaluateJavaScript('window.__testError'))
 
 
-class MediaCasesStorySet(story.StorySet):
+def _GetMediaPages(page_set):
+  return [
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=crowd.ogg&type=audio',
+          page_set=page_set,
+          tags=['vorbis', 'audio_only']),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=crowd1080.webm',
+          page_set=page_set,
+          tags=['is_50fps', 'vp8', 'vorbis', 'audio_video']),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=tulip2.ogg&type=audio',
+          page_set=page_set,
+          tags=['vorbis', 'audio_only']),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=tulip2.wav&type=audio',
+          page_set=page_set,
+          tags=['pcm', 'audio_only']),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=crowd1080.mp4',
+          page_set=page_set,
+          tags=['is_50fps', 'h264', 'aac', 'audio_video']),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=tulip2.mp3&type=audio',
+          page_set=page_set,
+          tags=['mp3', 'audio_only']),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=tulip2.mp4',
+          page_set=page_set,
+          tags=['h264', 'aac', 'audio_video']),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=tulip2.m4a&type=audio',
+          page_set=page_set,
+          tags=['aac', 'audio_only']),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=garden2_10s.webm',
+          page_set=page_set,
+          tags=['is_4k', 'vp8', 'vorbis', 'audio_video']),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=garden2_10s.mp4',
+          page_set=page_set,
+          tags=['is_4k', 'h264', 'aac', 'audio_video']),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=tulip2.vp9.webm',
+          page_set=page_set,
+          tags=['vp9', 'opus', 'audio_video'],
+          traffic_setting=traffic_setting_module.WIFI),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=tulip2.vp9.webm',
+          page_set=page_set,
+          tags=['vp9', 'opus', 'audio_video']),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=crowd1080_vp9.webm',
+          page_set=page_set,
+          tags=['vp9', 'video_only']),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=crowd720_vp9.webm',
+          page_set=page_set,
+          tags=['vp9', 'video_only']),
+      _BeginningToEndPlayPage(
+          url=_URL_BASE + 'video.html?src=tulip2.mp4&busyjs',
+          page_set=page_set,
+          tags=['h264', 'aac', 'audio_video', 'busyjs']),
+      _SeekPage(
+          url=_URL_BASE + 'video.html?src=tulip2.ogg&type=audio&seek',
+          page_set=page_set,
+          tags=['vorbis', 'audio_only']),
+      _SeekPage(
+          url=_URL_BASE + 'video.html?src=tulip2.wav&type=audio&seek',
+          page_set=page_set,
+          tags=['pcm', 'audio_only']),
+      _SeekPage(
+          url=_URL_BASE + 'video.html?src=tulip2.mp3&type=audio&seek',
+          page_set=page_set,
+          tags=['mp3', 'audio_only']),
+      _SeekPage(
+          url=_URL_BASE + 'video.html?src=tulip2.mp4&seek',
+          page_set=page_set,
+          tags=['h264', 'aac', 'audio_video']),
+      _SeekPage(
+          url=_URL_BASE + 'video.html?src=garden2_10s.webm&seek',
+          page_set=page_set,
+          tags=['is_4k', 'vp8', 'vorbis', 'audio_video']),
+      _SeekPage(
+          url=_URL_BASE + 'video.html?src=garden2_10s.mp4&seek',
+          page_set=page_set,
+          tags=['is_4k', 'h264', 'aac', 'audio_video']),
+      _SeekPage(
+          url=_URL_BASE + 'video.html?src=tulip2.vp9.webm&seek',
+          page_set=page_set,
+          tags=['vp9', 'opus', 'audio_video']),
+      _SeekPage(
+          url=_URL_BASE + 'video.html?src=crowd1080_vp9.webm&seek',
+          page_set=page_set,
+          tags=['vp9', 'video_only', 'seek']),
+      _SeekPage(
+          url=(_URL_BASE + 'video.html?src='
+               'smpte_3840x2160_60fps_vp9.webm&seek'),
+          page_set=page_set,
+          tags=['is_4k', 'vp9', 'video_only'],
+          action_timeout_in_seconds=120),
+      _BackgroundPlaybackPage(
+          url=_URL_BASE + 'video.html?src=tulip2.vp9.webm&background',
+          page_set=page_set,
+          tags=['vp9', 'opus', 'audio_video']),
+      _MSEPage(
+          url=_URL_BASE + 'mse.html?media=aac_audio.mp4,h264_video.mp4',
+          page_set=page_set,
+          tags=['h264', 'aac', 'audio_video']),
+      _MSEPage(
+          url=(_URL_BASE + 'mse.html?'
+               'media=aac_audio.mp4,h264_video.mp4&waitForPageLoaded=true'),
+          page_set=page_set,
+          tags=['h264', 'aac', 'audio_video']),
+      _MSEPage(
+          url=_URL_BASE + 'mse.html?media=aac_audio.mp4',
+          page_set=page_set,
+          tags=['aac', 'audio_only']),
+      _MSEPage(
+          url=_URL_BASE + 'mse.html?media=h264_video.mp4',
+          page_set=page_set,
+          tags=['h264', 'video_only']),
+      ]
+
+
+class _MediaCasesStorySet(story.StorySet):
+  """Abstract Media Cases Story Set to be overriden."""
+  def __init__(self, measure_memory=False):
+    super(_MediaCasesStorySet, self).__init__(
+            cloud_storage_bucket=story.PARTNER_BUCKET)
+    self.measure_memory = measure_memory
+    for page in self._BuildPages():
+      self.AddStory(page)
+
+  @abc.abstractmethod
+  def _BuildPages(self):
+    """Subclasses should implement this to return an iterator of pages."""
+
+
+class MediaCasesDesktopStorySet(_MediaCasesStorySet):
   """
   Description: Video Stack Perf pages that report time_to_play, seek time and
   many other media-specific and generic metrics.
   """
-  def __init__(self, measure_memory=False):
-    super(MediaCasesStorySet, self).__init__(
-            cloud_storage_bucket=story.PARTNER_BUCKET)
+  def _BuildPages(self):
+    return iter(_GetMediaPages(self))
 
-    self.measure_memory = measure_memory
 
-    pages = [
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=crowd.ogg&type=audio',
-            page_set=self,
-            tags=['vorbis', 'audio_only']),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=crowd1080.webm',
-            page_set=self,
-            tags=['is_50fps', 'vp8', 'vorbis', 'audio_video']),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=tulip2.ogg&type=audio',
-            page_set=self,
-            tags=['vorbis', 'audio_only']),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=tulip2.wav&type=audio',
-            page_set=self,
-            tags=['pcm', 'audio_only']),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=crowd1080.mp4',
-            page_set=self,
-            tags=['is_50fps', 'h264', 'aac', 'audio_video']),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=tulip2.mp3&type=audio',
-            page_set=self,
-            tags=['mp3', 'audio_only']),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=tulip2.mp4',
-            page_set=self,
-            tags=['h264', 'aac', 'audio_video']),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=tulip2.m4a&type=audio',
-            page_set=self,
-            tags=['aac', 'audio_only']),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=garden2_10s.webm',
-            page_set=self,
-            tags=['is_4k', 'vp8', 'vorbis', 'audio_video']),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=garden2_10s.mp4',
-            page_set=self,
-            tags=['is_4k', 'h264', 'aac', 'audio_video']),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=tulip2.vp9.webm',
-            page_set=self,
-            tags=['vp9', 'opus', 'audio_video'],
-            traffic_setting=traffic_setting_module.WIFI),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=tulip2.vp9.webm',
-            page_set=self,
-            tags=['vp9', 'opus', 'audio_video']),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=crowd1080_vp9.webm',
-            page_set=self,
-            tags=['vp9', 'video_only']),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=crowd720_vp9.webm',
-            page_set=self,
-            tags=['vp9', 'video_only']),
-        _BeginningToEndPlayPage(
-            url=_URL_BASE + 'video.html?src=tulip2.mp4&busyjs',
-            page_set=self,
-            tags=['h264', 'aac', 'audio_video', 'busyjs']),
-        _SeekPage(
-            url=_URL_BASE + 'video.html?src=tulip2.ogg&type=audio&seek',
-            page_set=self,
-            tags=['vorbis', 'audio_only']),
-        _SeekPage(
-            url=_URL_BASE + 'video.html?src=tulip2.wav&type=audio&seek',
-            page_set=self,
-            tags=['pcm', 'audio_only']),
-        _SeekPage(
-            url=_URL_BASE + 'video.html?src=tulip2.mp3&type=audio&seek',
-            page_set=self,
-            tags=['mp3', 'audio_only']),
-        _SeekPage(
-            url=_URL_BASE + 'video.html?src=tulip2.mp4&seek',
-            page_set=self,
-            tags=['h264', 'aac', 'audio_video']),
-        _SeekPage(
-            url=_URL_BASE + 'video.html?src=garden2_10s.webm&seek',
-            page_set=self,
-            tags=['is_4k', 'vp8', 'vorbis', 'audio_video']),
-        _SeekPage(
-            url=_URL_BASE + 'video.html?src=garden2_10s.mp4&seek',
-            page_set=self,
-            tags=['is_4k', 'h264', 'aac', 'audio_video']),
-        _SeekPage(
-            url=_URL_BASE + 'video.html?src=tulip2.vp9.webm&seek',
-            page_set=self,
-            tags=['vp9', 'opus', 'audio_video']),
-        _SeekPage(
-            url=_URL_BASE + 'video.html?src=crowd1080_vp9.webm&seek',
-            page_set=self,
-            tags=['vp9', 'video_only', 'seek']),
-        _SeekPage(
-            url=(_URL_BASE + 'video.html?src='
-                 'smpte_3840x2160_60fps_vp9.webm&seek'),
-            page_set=self,
-            tags=['is_4k', 'vp9', 'video_only'],
-            action_timeout_in_seconds=120),
-        _BackgroundPlaybackPage(
-            url=_URL_BASE + 'video.html?src=tulip2.vp9.webm&background',
-            page_set=self,
-            tags=['vp9', 'opus', 'audio_video']),
-        _MSEPage(
-            url=_URL_BASE + 'mse.html?media=aac_audio.mp4,h264_video.mp4',
-            page_set=self,
-            tags=['h264', 'aac', 'audio_video']),
-        _MSEPage(
-            url=(_URL_BASE + 'mse.html?'
-                 'media=aac_audio.mp4,h264_video.mp4&waitForPageLoaded=true'),
-            page_set=self,
-            tags=['h264', 'aac', 'audio_video']),
-        _MSEPage(
-            url=_URL_BASE + 'mse.html?media=aac_audio.mp4',
-            page_set=self,
-            tags=['aac', 'audio_only']),
-        _MSEPage(
-            url=_URL_BASE + 'mse.html?media=h264_video.mp4',
-            page_set=self,
-            tags=['h264', 'video_only']),
-    ]
-
-    for page in pages:
-      self.AddStory(page)
+class MediaCasesMobileStorySet(_MediaCasesStorySet):
+  """
+  Description: Video Stack Perf pages that report time_to_play, seek time and
+  many other media-specific and generic metrics.
+
+  The mobile story set removes stories that are too difficult for mobile
+  devices.
+  """
+  def _BuildPages(self):
+    for page in _GetMediaPages(self):
+      if 'is_4k' in page.tags or 'is_50fps' in page.tags:
+        continue
+      yield page