Bug 190266 - Add support for reporting "display composited video frames" through the VideoPlaybackQuality object.
Summary: Add support for reporting "display composited video frames" through the Video...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-10-03 15:31 PDT by Jer Noble
Modified: 2018-10-05 09:39 PDT (History)
9 users (show)

See Also:


Attachments
Patch (28.26 KB, patch)
2018-10-03 15:37 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (16.99 KB, patch)
2018-10-03 17:36 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (28.65 KB, patch)
2018-10-03 17:38 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (28.70 KB, patch)
2018-10-03 17:46 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (28.97 KB, patch)
2018-10-04 10:51 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (29.18 KB, patch)
2018-10-04 11:29 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (30.11 KB, patch)
2018-10-04 11:51 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (30.67 KB, patch)
2018-10-04 12:00 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (31.47 KB, patch)
2018-10-04 12:08 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-sierra (2.33 MB, application/zip)
2018-10-04 13:18 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews107 for mac-sierra-wk2 (2.96 MB, application/zip)
2018-10-04 13:28 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews117 for mac-sierra (3.47 MB, application/zip)
2018-10-04 14:00 PDT, EWS Watchlist
no flags Details
Patch for landing (31.80 KB, patch)
2018-10-04 14:32 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (31.83 KB, patch)
2018-10-04 16:11 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 2018-10-03 15:31:57 PDT
Add support for reporting "display composited video frames" through the VideoPlaybackQuality object.
Comment 1 Jer Noble 2018-10-03 15:37:36 PDT
Created attachment 351554 [details]
Patch
Comment 2 Jer Noble 2018-10-03 17:36:03 PDT
Created attachment 351565 [details]
Patch
Comment 3 Jer Noble 2018-10-03 17:38:04 PDT
Created attachment 351566 [details]
Patch
Comment 4 Jer Noble 2018-10-03 17:46:56 PDT
Created attachment 351568 [details]
Patch
Comment 5 Eric Carlson 2018-10-03 21:32:29 PDT
Comment on attachment 351568 [details]
Patch

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

r=me once the bots are happy

> Source/WebCore/Modules/mediasource/VideoPlaybackQuality.h:38
> +    static Ref<VideoPlaybackQuality> create(double creationTime, const VideoPlaybackQualityMetrics&);

VideoPlaybackQualityMetrics&& ?

> Source/WebCore/Modules/mediasource/VideoPlaybackQuality.h:48
> +    VideoPlaybackQuality(double creationTime, const VideoPlaybackQualityMetrics&);

Ditto
Comment 6 Jer Noble 2018-10-04 10:45:35 PDT
(In reply to Eric Carlson from comment #5)
> Comment on attachment 351568 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=351568&action=review
> 
> r=me once the bots are happy
> 
> > Source/WebCore/Modules/mediasource/VideoPlaybackQuality.h:38
> > +    static Ref<VideoPlaybackQuality> create(double creationTime, const VideoPlaybackQualityMetrics&);
> 
> VideoPlaybackQualityMetrics&& ?
> 
> > Source/WebCore/Modules/mediasource/VideoPlaybackQuality.h:48
> > +    VideoPlaybackQuality(double creationTime, const VideoPlaybackQualityMetrics&);
> 
> Ditto

This would imply that the VideoPlaybackQuality object was taking these values directly from the passed in struct. That might make sense if we were literally storing those values in an VideoPlaybackQualityMetrics ivar, but we only using the values themselves.
Comment 7 Jer Noble 2018-10-04 10:51:08 PDT
Created attachment 351600 [details]
Patch for landing
Comment 8 Jer Noble 2018-10-04 11:29:00 PDT
Created attachment 351606 [details]
Patch for landing
Comment 9 Jer Noble 2018-10-04 11:51:14 PDT
Created attachment 351608 [details]
Patch for landing
Comment 10 Jer Noble 2018-10-04 12:00:29 PDT
Created attachment 351609 [details]
Patch for landing
Comment 11 Jer Noble 2018-10-04 12:08:26 PDT
Created attachment 351610 [details]
Patch for landing
Comment 12 Jer Noble 2018-10-04 13:10:09 PDT
<rdar://problem/43948647>
Comment 13 EWS Watchlist 2018-10-04 13:18:35 PDT Comment hidden (obsolete)
Comment 14 EWS Watchlist 2018-10-04 13:18:37 PDT Comment hidden (obsolete)
Comment 15 EWS Watchlist 2018-10-04 13:28:54 PDT Comment hidden (obsolete)
Comment 16 EWS Watchlist 2018-10-04 13:28:56 PDT Comment hidden (obsolete)
Comment 17 EWS Watchlist 2018-10-04 14:00:52 PDT Comment hidden (obsolete)
Comment 18 EWS Watchlist 2018-10-04 14:00:54 PDT Comment hidden (obsolete)
Comment 19 Jer Noble 2018-10-04 14:32:12 PDT
Created attachment 351633 [details]
Patch for landing
Comment 20 Jer Noble 2018-10-04 16:11:51 PDT
Created attachment 351640 [details]
Patch for landing
Comment 21 WebKit Commit Bot 2018-10-04 18:11:30 PDT
Comment on attachment 351640 [details]
Patch for landing

Clearing flags on attachment: 351640

Committed r236866: <https://trac.webkit.org/changeset/236866>
Comment 22 WebKit Commit Bot 2018-10-04 18:11:32 PDT
All reviewed patches have been landed.  Closing bug.
Comment 23 Antoine Quint 2018-10-04 23:27:29 PDT
My debug build failed because of this commit:

In file included from /Users/antoine/Code/safari/OpenSource/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:38:
PAL/pal/spi/mac/AVFoundationSPI.h:191:4: error: expected a type
- (AVVideoPerformanceMetrics *)videoPerformanceMetrics;
   ^
PAL/pal/spi/mac/AVFoundationSPI.h:191:1: error: method has no return type specified; defaults to 'id' [-Werror,-Wmissing-method-return-type]
- (AVVideoPerformanceMetrics *)videoPerformanceMetrics;
^
2 errors generated.
Comment 24 Chris Dumez 2018-10-05 08:50:42 PDT
(In reply to Antoine Quint from comment #23)
> My debug build failed because of this commit:
> 
> In file included from
> /Users/antoine/Code/safari/OpenSource/Source/WebCore/platform/graphics/
> avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:38:
> PAL/pal/spi/mac/AVFoundationSPI.h:191:4: error: expected a type
> - (AVVideoPerformanceMetrics *)videoPerformanceMetrics;
>    ^
> PAL/pal/spi/mac/AVFoundationSPI.h:191:1: error: method has no return type
> specified; defaults to 'id' [-Werror,-Wmissing-method-return-type]
> - (AVVideoPerformanceMetrics *)videoPerformanceMetrics;
> ^
> 2 errors generated.

Same here.
Comment 25 Ryan Haddad 2018-10-05 08:58:47 PDT
Reverted r236866 for reason:

Breaks internal builds.

Committed r236874: <https://trac.webkit.org/changeset/236874>
Comment 26 Jer Noble 2018-10-05 09:04:52 PDT
Committed r236875: <https://trac.webkit.org/changeset/236875>
Comment 27 Jer Noble 2018-10-05 09:39:49 PDT
Committed build fix in r236876: <https://trac.webkit.org/changeset/236876>