Bug 175835

Summary: Track VideoPlaybackQuality metrics when using WebCoreDecompressionSession.
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: MediaAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, eric.carlson, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews103 for mac-elcapitan
none
Archive of layout-test-results from ews117 for mac-elcapitan
none
Patch
none
Patch for landing
none
Patch none

Description Jer Noble 2017-08-22 11:06:47 PDT
Return VideoPlaybackQuality metrics when using WebCoreDecompressionSession.
Comment 1 Radar WebKit Bug Importer 2017-08-22 14:56:39 PDT
<rdar://problem/34022234>
Comment 2 Jer Noble 2017-08-22 15:00:30 PDT
Created attachment 318803 [details]
Patch
Comment 3 Build Bot 2017-08-22 15:54:47 PDT
Comment on attachment 318803 [details]
Patch

Attachment 318803 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4364060

New failing tests:
platform/mac/media/media-source/videoplaybackquality-decompressionsession.html
media/modern-media-controls/seek-backward-support/seek-backward-support.html
Comment 4 Build Bot 2017-08-22 15:54:49 PDT
Created attachment 318815 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 5 Build Bot 2017-08-22 17:08:40 PDT
Comment on attachment 318803 [details]
Patch

Attachment 318803 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4364459

New failing tests:
platform/mac/media/media-source/videoplaybackquality-decompressionsession.html
Comment 6 Build Bot 2017-08-22 17:08:42 PDT
Created attachment 318826 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 7 Eric Carlson 2017-08-23 10:26:09 PDT
Comment on attachment 318803 [details]
Patch

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

> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:265
> +    if (status != noErr || noErr != CMVideoFormatDescriptionCreateForImageBuffer(kCFAllocatorDefault, rawImageBuffer, &rawImageBufferDescription)) {

Nit: noErr is 0 so you don't need "!= noErr"

> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:278
> +    if (noErr != CMSampleBufferCreateReadyWithImageBuffer(kCFAllocatorDefault, rawImageBuffer, imageBufferDescription.get(), &imageBufferTiming, &rawImageSampleBuffer)) {

Ditto.
Comment 8 Jer Noble 2017-08-23 10:57:48 PDT
(In reply to Eric Carlson from comment #7)
> Comment on attachment 318803 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=318803&action=review
> 
> > Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:265
> > +    if (status != noErr || noErr != CMVideoFormatDescriptionCreateForImageBuffer(kCFAllocatorDefault, rawImageBuffer, &rawImageBufferDescription)) {
> 
> Nit: noErr is 0 so you don't need "!= noErr"
> 
> > Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:278
> > +    if (noErr != CMSampleBufferCreateReadyWithImageBuffer(kCFAllocatorDefault, rawImageBuffer, imageBufferDescription.get(), &imageBufferTiming, &rawImageSampleBuffer)) {
> 
> Ditto.

We check against noErr everywhere in WebKit and WebCore. It might evaluate to zero, but I think the style guide is strictly about comparing directly to zero.
Comment 9 Jer Noble 2017-08-23 10:58:30 PDT
Created attachment 318887 [details]
Patch
Comment 10 Jer Noble 2017-08-23 12:43:50 PDT
Created attachment 318902 [details]
Patch for landing
Comment 11 Jer Noble 2017-08-23 12:54:01 PDT
Created attachment 318904 [details]
Patch
Comment 12 WebKit Commit Bot 2017-08-23 13:16:57 PDT
Comment on attachment 318902 [details]
Patch for landing

Clearing flags on attachment: 318902

Committed r221098: <http://trac.webkit.org/changeset/221098>
Comment 13 WebKit Commit Bot 2017-08-23 13:16:59 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Jer Noble 2017-08-23 13:42:22 PDT
Follow-up build fix:

Committed r221100: <http://trac.webkit.org/changeset/r221100>