Bug 175835 – Track VideoPlaybackQuality metrics when using WebCoreDecompressionSession.

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jer Noble]

Return VideoPlaybackQuality metrics when using WebCoreDecompressionSession.

[Radar WebKit Bug Importer]

<rdar://problem/34022234>

[Jer Noble]

Created attachment 318803 [details]
Patch

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Eric Carlson]

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.

[Jer Noble]

(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.

[Jer Noble]

Created attachment 318887 [details]
Patch

[Jer Noble]

Created attachment 318902 [details]
Patch for landing

[Jer Noble]

Created attachment 318904 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 318902 [details]
Patch for landing

Clearing flags on attachment: 318902

Committed r221098: <http://trac.webkit.org/changeset/221098>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Jer Noble]

Follow-up build fix:

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