Cast: Prefer VP8 over H.264 for hardware encoder

Cast: Prefer VP8 over H.264 for hardware encoder

BUG=469409
TEST=Test on Nyan, which has both H.264 and VP8 HW encoder.
     It should choose VP8 instead of H.264.

Committed: https://crrev.com/2da6f193662ce92fe80641b2ec595aa822d939e9
Cr-Commit-Position: refs/heads/master@{#324374}

[Justin Chuang, 2015-03-24 06:05:34]

jchuang@chromium.org changed reviewers:
+ hshi@chromium.org, miu@chromium.org

[Justin Chuang, 2015-03-24 06:05:34]

PTAL. Thanks

[Justin Chuang, 2015-03-25 03:26:45]

On 2015/03/24 06:05:34, Justin Chuang wrote:
> PTAL. Thanks

This mainly impacts Exynos and Tegra platform, which has both H.264 and VP8 HW
encoder. The original code picks H.264 over VP8.

[miu, 2015-03-27 20:41:06]

Sorry for the delay.  I assume, from a quality and user-experience perspective,
using the HW VP8 encoder is equal-or-better than using the HW H.264 encoder?

Comments:

https://codereview.chromium.org/1034433002/diff/1/chrome/renderer/media/cast_...
File chrome/renderer/media/cast_rtp_stream.cc (right):

https://codereview.chromium.org/1034433002/diff/1/chrome/renderer/media/cast_...
chrome/renderer/media/cast_rtp_stream.cc:156:
supported_params.push_back(CastRtpParams(DefaultVp8Payload()));
It's fine to prefer the VP8 HW encoder, but I think you should also include
H.264 as a second option, to let an extension decide which to use.  Meaning,
this method should be:

std::vector<CastRtpParams> SupportedVideoParams() {
  std::vector<CastRtpParams> supported_params;

  if (IsHardwareVP8EncodingSupported())
    supported_params.push_back(CastRtpParams(DefaultVp8Payload()));
  if (IsHardwareH264EncodingSupported())
    supported_params.push_back(CastRtpParams(DefaultH264Payload()));

  // Propose the default software VP8 encoder, if no hardware encoders are
available.
  if (supported_params.empty())
    supported_params.push_back(CastRtpParams(DefaultVp8Payload()));

  return supported_params;
}

[hubbe, 2015-03-27 21:08:01]

hubbe@chromium.org changed reviewers:
+ hubbe@chromium.org

[hubbe, 2015-03-27 21:08:01]

I've recently learned that some VP8 hardware codecs are not able to encode
high-resolution video because they are simply not fast enough. I don't want nix
this change, but I too am curious if the impact of this change is known or not.

[Justin Chuang, 2015-03-31 07:09:38]

jchuang@chromium.org changed reviewers:
+ posciak@chromium.org

[Justin Chuang, 2015-03-31 07:09:38]

+ Pawel

[Justin Chuang, 2015-03-31 10:22:54]

On 2015/03/27 20:41:06, miu wrote:
> Sorry for the delay.  I assume, from a quality and user-experience
perspective,
> using the HW VP8 encoder is equal-or-better than using the HW H.264 encoder?
 
AFAIK, VP8 has less decode latency because it doesn't have frames equivalent to
B-frame in H.264. 

Pawel, please correct it if I'm wrong.

> Comments:
> It's fine to prefer the VP8 HW encoder, but I think you should also include
> H.264 as a second option, to let an extension decide which to use.  Meaning,
> this method should be:
> 
Thanks!

[Justin Chuang, 2015-03-31 10:25:29]

On 2015/03/27 21:08:01, hubbe wrote:
> I've recently learned that some VP8 hardware codecs are not able to encode
> high-resolution video because they are simply not fast enough. I don't want
nix
> this change, but I too am curious if the impact of this change is known or
not.

No, sorry. I was not aware of it. 
I can verify known impacted CrOS platform, but I don't know which Android SoC
has slow VP8 HW codecs.

[Pawel Osciak, 2015-03-31 11:38:10]

On 2015/03/31 10:22:54, Justin Chuang wrote:
> On 2015/03/27 20:41:06, miu wrote:
> > Sorry for the delay.  I assume, from a quality and user-experience
> perspective,
> > using the HW VP8 encoder is equal-or-better than using the HW H.264 encoder?
>  
> AFAIK, VP8 has less decode latency because it doesn't have frames equivalent
to
> B-frame in H.264. 
> 
> Pawel, please correct it if I'm wrong.
> 

B-frames are not normally used in casting/realtime scenarios for this reason. So
this wouldn't be the case.

[Pawel Osciak, 2015-03-31 11:45:30]

On 2015/03/31 10:25:29, Justin Chuang wrote:
> On 2015/03/27 21:08:01, hubbe wrote:
> > I've recently learned that some VP8 hardware codecs are not able to encode
> > high-resolution video because they are simply not fast enough. I don't want
> nix
> > this change, but I too am curious if the impact of this change is known or
> not.
> 
> No, sorry. I was not aware of it. 
> I can verify known impacted CrOS platform, but I don't know which Android SoC
> has slow VP8 HW codecs.

Depends on what high-resolution is in this case, 1080p? It's likely that older
(but we are talking probably 3 years+) SoCs can't do 1080p encode @30fps, but
then I'd expect them to not be able to do H264 at that level either.

I think the more likely reason for this could be due to older platforms actually
not having VP8 HW encoders at all, while H264 has been incomparably more
prevalent in the past. If that's the case, I think we should be fine if we
selected VP8 only if we had HW encode for it, and if not, fell back, which is
what IsHardwareVP8EncodingSupported() here should be doing.

But we also have max resolutions/fps supported by the codec already provided in
the API, so we could handle all cases and cover all bases if we made
IsHardwareVP8EncodingSupported() smarter and also verified max resolution that
the encoder could handle. What do you think?

[miu, 2015-03-31 19:07:03]

Hubbe also mentioned to me that sometimes the VP8 HW encoders produce poor
quality results compared to H.264 in the older machines.  Sounds like we need to
do some research/testing and find out how smart the SupportedVideoParams()
function needs to be.

[Justin Chuang, 2015-04-07 08:10:24]

On 2015/03/31 19:07:03, miu wrote:
> Hubbe also mentioned to me that sometimes the VP8 HW encoders produce poor
> quality results compared to H.264 in the older machines.  Sounds like we need
to
> do some research/testing and find out how smart the SupportedVideoParams()
> function needs to be.

Thanks. List all platforms impacted as below:

[A] Chrome OS:
1. x86 (Broadwell, Haswell, Baytrail): They do not have HW VP8 encoder. (Some
have VP8 HW decoder).
2. ARM platform: We support three ARM SoC families with HW VP8 encoder (Exynos
5420 and later, Tegra 124 and later, Rk). All three SoC families are new (< 3
years) and have acceptable VP8 quality. 

[B] Mac OS 
Only supporting H.264, no VP8.

[C] Windows
1. x86 platform hasn't have VP8 HW encoder. (Skylake will probably support HW
VP8 encoder, but it's not an issue.)
2. ARM platform (Win8 RT) is not supported in DXVAVideoDecodeAccelerator, and I
don't find VEA on Windows. 
So no VP8 HW encoder on Windows now.

[D] Android
Since there is Chromecast app, do we need to support Cast extension for Chrome
on Android?

If no support of Cast extension for Android, the only impacted platforms are the
three ARM SoC families on ChromeOS now. 
On these SoC, VP8 is preferred over H.264.

[miu, 2015-04-08 00:38:15]

The CQ bit was checked by miu@chromium.org

[miu, 2015-04-08 00:38:15]

lgtm

Thanks for looking into the affected platforms.

[commit-bot: I haz the power, 2015-04-08 00:39:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1034433002/20001

[commit-bot: I haz the power, 2015-04-08 00:49:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-08 00:49:47]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Justin Chuang, 2015-04-08 12:32:53]

Hubbe, please help owner review. Thanks

[hubbe, 2015-04-08 19:30:26]

LGTM

thanks for looking into the effects

[miu, 2015-04-08 20:26:05]

The CQ bit was checked by miu@chromium.org

[miu, 2015-04-08 20:26:17]

The CQ bit was unchecked by miu@chromium.org

[Justin Chuang, 2015-04-09 03:44:10]

The CQ bit was checked by jchuang@chromium.org

[commit-bot: I haz the power, 2015-04-09 03:45:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1034433002/20001

[commit-bot: I haz the power, 2015-04-09 04:44:14]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-09 04:45:59]

Patchset 2 (id:??) landed as
https://crrev.com/2da6f193662ce92fe80641b2ec595aa822d939e9
Cr-Commit-Position: refs/heads/master@{#324374}