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

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>