Add SetVideoMode API to avsettings

chromecast/public/avsettings.h

Index: chromecast/public/avsettings.h
diff --git a/chromecast/public/avsettings.h b/chromecast/public/avsettings.h
index ca3da29cf45e3ac4b5efd82267736081b0131c07..82963aaca1fe3cc1fd29a96f1e17cf5c4e0ac18e 100644
--- a/chromecast/public/avsettings.h
+++ b/chromecast/public/avsettings.h
@@ -276,6 +276,28 @@ class AvSettings {
   // Enables/Disables Wake-On-Cast status.
   // Returns false if failed or not supported.
   virtual bool EnableWakeOnCast(bool enabled) = 0;
+
+  // Sets the video mode according to the given parameters:
+  //
+  // allow_4k: if false, the resolution set will not be a 4K resolution.
+  //
+  // prefer_50hz: picks a resolution optimized for 50 hz content.

[halliwell, 2016/09/30 21:28:14]

can we eliminate this in public API (use the fps parameter instead).  I know we
might be stuck with that internally but the public API is harder to change.

[almasrymina, 2016/10/03 20:05:05]

Done.

+  //
+  // optimize_for_fps: *Attempts* to pick an fps optimal for the given fps.

[halliwell, 2016/09/30 21:28:14]

"pick a refresh rate optimal for the given content frame rate"

[almasrymina, 2016/10/03 20:05:05]

Done.

+  // Values <= 0 are ignored.
+  //
+  // Returns false if it will not attempt to set the video mode. This could be
+  // due to:
+  // HDMI state is not connected.
+  // Resolution set is already the best resolution with the params provided.

[halliwell, 2016/09/30 21:28:14]

'resolution is already best' should not be the same return value as error case.

[almasrymina, 2016/10/03 20:05:05]

Discussed offline: the error cases here are not things the app care or can do
anything about, so the design is around this:

if the API returns true, wait for a SCREEN_INFO_CHANGED event before proceeding.
If the API returns false, go ahead and start playback without waiting.

I could change this so it returns false in the HDMI disconnected case, and true
in the 'Resolution already set' case, and fire the event always, but that
doesn't seem nice to me. I don't imagine the app cares if HDMI is connected or
not, the app will probably want to start playback anyway since the user is
asking for that. ANd if the app cares about whether we're connected or not, we
do fire HDMI_CONNECTED and DISCONNECTED events and they should use that and not
the return value here.

+  //
+  // Returns True otherwise. A SCREEN_INFO_CHANGED event fires when the video

[halliwell, 2016/09/30 21:28:14]

nit: 'true'

[almasrymina, 2016/10/03 20:05:05]

Done.

+  // mode setting completes.
+  virtual bool SetVideoMode(bool prefer_50hz,
+                            bool allow_4k,
+                            int optimize_for_fps) {

[halliwell, 2016/09/30 21:28:14]

can we make this a float (or express as fps*100) so we can distinguish e.g.
29.97 from 30fps

[almasrymina, 2016/10/03 20:05:05]

Done.

+    return false;
+  }

[gfhuang, 2016/09/30 20:30:24]

public API must be pure.

and from the comment, it's not clear to me how does this apply to CastTV and
AndroidTV.

[halliwell, 2016/09/30 21:08:23]

I actually wonder if we should group the HDMI-related functions in a separate
HdmiSettings interface (AvSettings could have GetHdmi function to access it, and
Cast TV could just return null, then they can ignore all HDMI_related changes).

Thoughts?

[almasrymina, 2016/10/03 20:05:05]

Done.

 };
 
 }  // namespace chromecast