Bug 53348 - Implement media statistics
Summary: Implement media statistics
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-28 16:04 PST by Steve Lacey
Modified: 2011-02-02 11:58 PST (History)
9 users (show)

See Also:


Attachments
Patch (33.03 KB, patch)
2011-01-28 17:48 PST, Steve Lacey
no flags Details | Formatted Diff | Diff
Patch (33.07 KB, patch)
2011-02-01 12:43 PST, Steve Lacey
eric.carlson: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Lacey 2011-01-28 16:04:41 PST
Add webkit{Decoded,Dropped}Frames to video element.
Add webkit{Audio,Video}BytesDecoded to media element.
Comment 1 Steve Lacey 2011-01-28 17:48:15 PST
Created attachment 80530 [details]
Patch
Comment 2 Steve Lacey 2011-01-28 17:51:34 PST
The trickiest bit in all this was finding everything I needed to change to add a new feature define ;-)

I believe the configure.ac and GNUMakefile.am changes will be tested by the gtk bots?

A potential modification to this would be to move the {audio,video}BytesDecoded attributes to the video element and add a pure bytesDecoded to the audio element. Thoughts?

I'm still not sure what the best thing to do when the data is not available (which shouldn't actually be the case now as it's turned off in all ports by default).
Comment 3 Eric Seidel (no email) 2011-01-30 04:04:19 PST
Very interesting.  Looks sane to me, but CCing others who might have more context about APUI counters like these.
Comment 4 Eric Carlson 2011-01-31 17:34:05 PST
Comment on attachment 80530 [details]
Patch

Looks good overall, but I am torn about having the compile flag *only* checked in the IDL. We only need to hide the properties from JavaScript for a port that doesn't implement this, and I don't think it is necessary to surround all of the code in every class, I think it could be useful to have it in HTMLMediaElement so it is clear to anyone looking at the code that it is a compile time feature.
Comment 5 Steve Lacey 2011-01-31 17:52:41 PST
Thanks!

Am a little bit unclear from your comment whether you'd like me to add #if's to the implementation files or not?

Also, am not sure how to test the configure.ac, GNUMakefile.am or vsprops changes. Do the bots do that?
Comment 6 Eric Carlson 2011-02-01 08:14:15 PST
(In reply to comment #5)
> Thanks!
> 
> Am a little bit unclear from your comment whether you'd like me to add #if's to the implementation files or not?
> 
Sorry, my comment is completely unclear. I would prefer to have the #if in HTMLMediaElement.cpp/.h so it is easy to see that it is an opt-in feature. I don't have strong feelings about MediaPlayer and friends, either is OK.

> Also, am not sure how to test the configure.ac, GNUMakefile.am or vsprops changes. Do the bots do that?
You will have to babysit the build bots once the change lands to see if you broke something.
Comment 7 Steve Lacey 2011-02-01 12:43:49 PST
Created attachment 80805 [details]
Patch
Comment 8 Eric Carlson 2011-02-01 12:45:52 PST
Comment on attachment 80805 [details]
Patch

Thanks!
Comment 9 Steve Lacey 2011-02-01 15:08:23 PST
I'm still unclear about why the patch is failing to apply on the gtk-ews bot. Any clues?

If this is ok. Can I get a cq+ :-) Or is there some other process I'm missing (new to this...)

Thx.
Comment 10 WebKit Commit Bot 2011-02-01 15:53:37 PST
Comment on attachment 80805 [details]
Patch

Rejecting attachment 80805 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-..." exit_code: 2

Last 500 characters of output:
 of 2 hunks FAILED -- saving rejects to file WebKitLibraries/win/tools/vsprops/FeatureDefines.vsprops.rej
patching file WebKitLibraries/win/tools/vsprops/FeatureDefinesCairo.vsprops
Hunk #1 FAILED at 9.
Hunk #2 FAILED at 197.
2 out of 2 hunks FAILED -- saving rejects to file WebKitLibraries/win/tools/vsprops/FeatureDefinesCairo.vsprops.rej
patching file configure.ac

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Eric Carlson', u'--for..." exit_code: 1

Full output: http://queues.webkit.org/results/7685714
Comment 11 Steve Lacey 2011-02-01 16:10:33 PST
Sigh. I have really no idea why the patch to those two files would fail to apply as they haven't been modified in 3 months.

Will do some digging, but if anyone has a flash of inspiration about what I might be doing wrong I'd appreciate it...
Comment 12 Adam Barth 2011-02-01 16:15:16 PST
Dan was telling me that it might have something to do with line endings.  In any case, I'm sure its not your fault.  Someone should be able to help you land your patch manually (all my working copies are busy at the moment...).
Comment 13 Daniel Bates 2011-02-01 16:21:20 PST
(In reply to comment #12)
> Dan was telling me that it might have something to do with line endings.  In any case, I'm sure its not your fault.  Someone should be able to help you land your patch manually (all my working copies are busy at the moment...).

I'll take a look shortly.
Comment 14 Daniel Bates 2011-02-02 11:43:46 PST
(In reply to comment #13)
> (In reply to comment #12)
> > Dan was telling me that it might have something to do with line endings.  In any case, I'm sure its not your fault.  Someone should be able to help you land your patch manually (all my working copies are busy at the moment...).
> 
> I'll take a look shortly.

Weird. I was able to apply the patch using svn-apply without issue. Is the commit-queue bot running under Windows?
Comment 15 Daniel Bates 2011-02-02 11:44:34 PST
I'll land this by hand for Steve Lacey.
Comment 16 Daniel Bates 2011-02-02 11:46:22 PST
(In reply to comment #14)
> Weird. I was able to apply the patch using svn-apply without issue. Is the commit-queue bot running under Windows?

Disregard this remark. I think something is up with patch(1) on GTK-ews bot.
Comment 17 Daniel Bates 2011-02-02 11:58:25 PST
Committed in changeset 77394 <http://trac.webkit.org/changeset/77394>.