Bug 175835 - Track VideoPlaybackQuality metrics when using WebCoreDecompressionSession.
Summary: Track VideoPlaybackQuality metrics when using WebCoreDecompressionSession.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-22 11:06 PDT by Jer Noble
Modified: 2017-08-23 13:42 PDT (History)
5 users (show)

See Also:


Attachments
Patch (12.16 KB, patch)
2017-08-22 15:00 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-elcapitan (1.27 MB, application/zip)
2017-08-22 15:54 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews117 for mac-elcapitan (1.93 MB, application/zip)
2017-08-22 17:08 PDT, Build Bot
no flags Details
Patch (15.08 KB, patch)
2017-08-23 10:58 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (19.00 KB, patch)
2017-08-23 12:43 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (2.49 KB, patch)
2017-08-23 12:54 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>