[webkit-reviews] review granted: [Bug 226219] Support H264 profiles in MediaRecorder : [Attachment 429652] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted youenn fablet
<youennf at gmail.com>'s request for review:
Bug 226219: Support H264 profiles in MediaRecorder
https://bugs.webkit.org/show_bug.cgi?id=226219

Attachment 429652: Patch

https://bugs.webkit.org/attachment.cgi?id=429652&action=review




--- Comment #4 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 429652
  --> https://bugs.webkit.org/attachment.cgi?id=429652
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=429652&action=review

> Source/WebCore/ChangeLog:11
> +	   By default, use baseline profile, which is not VideoToolbox profile
but has wider decoding support.

I'm not sure what you mean by "which is not VideoToolbox profile but has wider
decoding support", it sounds like you are saying that VideoToolbox doesn't
support baseline.

> Source/WebCore/platform/mediarecorder/cocoa/VideoSampleBufferCompressor.mm:52
> +	   if (startsWithLettersIgnoringASCIICase(codec, "avc1.") &&
codec.length() >= 11) {
> +	       if (codec[5] == '6' && codec[6] == '4')
> +		   profile = Profile::High;
> +	       else if (codec[5] == '4' && (codec[6] == 'd' || codec[6] ==
'D'))
> +		   profile = Profile::Main;
> +	       break;

Do we want to use Baseline if the configuration is not supported or invalid? It
is probably worth logging in that case whatever we do.

>
Source/WebCore/platform/mediarecorder/cocoa/VideoSampleBufferCompressor.mm:165
> +    RELEASE_LOG_ERROR_IF(error, MediaStream, "VideoSampleBufferCompressor
VTSessionSetProperty kVTCompressionPropertyKey_ProfileLevel failed with %d",
error);

Probably with logging what profile failed.