Bug 137533 - MediaPlayer::characteristicChanged() is not called when new tracks are found in SourceBufferPrivateAVFObjC
Summary: MediaPlayer::characteristicChanged() is not called when new tracks are found ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-10-08 12:31 PDT by Ada Chan
Modified: 2014-12-16 13:28 PST (History)
9 users (show)

See Also:

Attachments
Patch (3.54 KB, patch)
2014-10-08 13:31 PDT, Jer Noble 		no flags Details | Formatted Diff | Diff
Patch for landing (108.96 KB, patch)
2014-10-11 00:55 PDT, Jer Noble 		no flags Details | Formatted Diff | Diff
Patch (4.81 KB, patch)
2014-10-11 10:48 PDT, Jer Noble 		no flags Details | Formatted Diff | Diff
Patch (4.81 KB, patch)
2014-10-11 23:23 PDT, Jer Noble 		no flags Details | Formatted Diff | Diff
Follow up patch (3.07 KB, patch)
2014-10-13 14:20 PDT, Jer Noble 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ada Chan 2014-10-08 12:31:07 PDT 
When new audio/video tracks are parsed in SourceBufferPrivateAVFObjC, I would expect MediaPlayer::characteristicsChanged() to be called since the media's characteristics (such as hasAudio, hasVideo) have changed.

1. Open a youtube video page showing HTML5 video.
2. Place a breakpoint in MediaPlayer::characteristicsChanged().

RESULTS: I don't see that called due to a change in hasAudio/hasVideo characteristics.
Comment 1 Jer Noble 2014-10-08 13:31:46 PDT 
Created attachment 239491 [details]
Patch
Comment 2 Darin Adler 2014-10-09 14:50:05 PDT 
Comment on attachment 239491 [details]
Patch

No regression test possible?
Comment 3 Jer Noble 2014-10-10 23:48:12 PDT 
(In reply to comment #2)
> (From update of attachment 239491 [details])
> No regression test possible?

A TestWebKitAPI unit test should be possible.  I'm working on one now.
Comment 4 Jer Noble 2014-10-11 00:55:20 PDT 
Created attachment 239668 [details]
Patch for landing
Comment 5 Jer Noble 2014-10-11 10:48:07 PDT 
Created attachment 239680 [details]
Patch

Also unregister for availability changes when the page is hidden (e.g., when in a background tab).
Comment 6 Jer Noble 2014-10-11 23:23:09 PDT 
Created attachment 239701 [details]
Patch
Comment 7 Eric Carlson 2014-10-12 08:31:43 PDT 
(In reply to comment #6)
> Created an attachment (id=239701) [details]
> Patch

I think you meant to attach this to bug 137633.
Comment 8 Jer Noble 2014-10-13 10:49:18 PDT 
Committed r174652: <http://trac.webkit.org/changeset/174652>
Comment 9 Andy Estes 2014-10-13 13:01:21 PDT 
Looks like the new API test is timing out on both the Mavericks and Mountain Lion bots. For example:

https://build.webkit.org/builders/Apple%20Mavericks%20Debug%20WK1%20%28Tests%29/builds/8217/steps/run-api-tests/logs/stdio
Comment 10 Jer Noble 2014-10-13 13:30:45 PDT 
Oof. MSE is only enabled on Yosemite.
Comment 11 Jer Noble 2014-10-13 14:20:04 PDT 
Reopening to attach new patch.
Comment 12 Jer Noble 2014-10-13 14:20:07 PDT 
Created attachment 239744 [details]
Follow up patch
Comment 13 Andy Estes 2014-10-13 14:22:43 PDT 
Comment on attachment 239744 [details]
Follow up patch

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

> Tools/TestWebKitAPI/Tests/WebKit2/WKPageIsPlayingAudio.cpp:58
> +    EXPECT_TRUE(JSValueIsBoolean(scriptContext, resultValue));

Won't this cause the test to fail on unsupported platforms?
Comment 14 Andy Estes 2014-10-13 14:24:19 PDT 
Comment on attachment 239744 [details]
Follow up patch

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

>> Tools/TestWebKitAPI/Tests/WebKit2/WKPageIsPlayingAudio.cpp:58
>> +    EXPECT_TRUE(JSValueIsBoolean(scriptContext, resultValue));
> 
> Won't this cause the test to fail on unsupported platforms?

Oh never mind. Sigh. IsBoolean != ToBoolean.
Comment 15 WebKit Commit Bot 2014-10-13 18:16:28 PDT 
Comment on attachment 239744 [details]
Follow up patch

Clearing flags on attachment: 239744

Committed r174667: <http://trac.webkit.org/changeset/174667>
Comment 16 WebKit Commit Bot 2014-10-13 18:16:32 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 17 Jer Noble 2014-12-16 13:28:11 PST 
<rdar://problem/19269053>