Bug 36624 - Configure multi-language movies
Summary: Configure multi-language movies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords: HTML5, InRadar
Depends on:
Blocks:
 
Reported: 2010-03-25 15:52 PDT by Jer Noble
Modified: 2010-04-02 16:33 PDT (History)
5 users (show)

See Also:


Attachments
Patch (4.81 KB, patch)
2010-03-25 17:09 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (5.39 KB, patch)
2010-03-26 09:29 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (6.30 KB, patch)
2010-03-30 17:12 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
New libWebKitSystemInterfaceLeopard.a (2.45 MB, application/octet-stream)
2010-03-30 17:14 PDT, Jer Noble
no flags Details
New libWebKitSystemInterfaceSnowLeopard.a (1.94 MB, application/octet-stream)
2010-03-30 17:15 PDT, Jer Noble
no flags Details
New libWebKitSystemInterfaceTiger.a (924.56 KB, application/octet-stream)
2010-03-30 17:16 PDT, Jer Noble
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2010-03-25 15:52:40 PDT
Configure multi-language movies
Comment 1 Jer Noble 2010-03-25 17:09:56 PDT
Created attachment 51694 [details]
Patch
Comment 2 Eric Seidel (no email) 2010-03-26 00:55:19 PDT
Comment on attachment 51694 [details]
Patch

Looks non-harmful.  Marking cq+ since you're not a committer.
Comment 3 Eric Seidel (no email) 2010-03-26 01:15:02 PDT
Comment on attachment 51694 [details]
Patch

Removing the cq+ since this might require a new WebCoreSystemInterface library drop.
Comment 4 WebKit Commit Bot 2010-03-26 01:23:56 PDT
Comment on attachment 51694 [details]
Patch

Rejecting patch 51694 from commit-queue.

Failed to run "['WebKitTools/Scripts/build-webkit', '--debug']" exit_code: 1
Last 500 characters of output:
/usr/bin/yacc
    /bin/sh -c /Users/eseidel/Projects/CommitQueue/WebKitBuild/WebKit.build/Debug/WebKit.build/Script-5DE6D18C0FCF231B002DE28C.sh
** BUILD FAILED **

The following build commands failed:
WebKit:
	Distributed-CompileC /Users/eseidel/Projects/CommitQueue/WebKitBuild/WebKit.build/Debug/WebKit.build/Objects-normal/i386/WebSystemInterface.o /Users/eseidel/Projects/CommitQueue/WebKit/mac/WebCoreSupport/WebSystemInterface.m normal i386 objective-c com.apple.compilers.gcc.4_2
(1 failure)


Full output: http://webkit-commit-queue.appspot.com/results/1326013
Comment 5 Eric Seidel (no email) 2010-03-26 01:26:46 PDT
I need to teach the commit-queue how to re-check the commit-queue flag just before landing.  In this case the bot had already started on the patch before I removed the flag.  Sorry for the noise.
Comment 6 Eric Carlson 2010-03-26 07:51:44 PDT
Comment on attachment 51694 [details]
Patch

> +++ WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm	(working copy)
> @@ -927,6 +927,7 @@ void MediaPlayerPrivate::updateStates()
>  
>      if (loadState >= QTMovieLoadStateLoaded && m_readyState < MediaPlayer::HaveMetadata) {
>          disableUnsupportedTracks();
> +        wkQTMovieSelectPreferredAlternates(m_qtMovie.get());
>          if (m_player->inMediaDocument()) {
>              if (!m_enabledTrackCount || m_hasUnsupportedTracks) {
>                  // This has a type of media that we do not handle directly with a <video> 

We don't know if the movie is playable at this point so you should call wkQTMovieSelectPreferredAlternates a few lines further down, after checking to see if the movie opened successfully (near the call to cacheMovieScale).


> Index: WebCore/platform/mac/WebCoreSystemInterface.h
> ===================================================================
> --- WebCore/platform/mac/WebCoreSystemInterface.h	(revision 56339)
> +++ WebCore/platform/mac/WebCoreSystemInterface.h	(working copy)
> @@ -129,6 +129,7 @@ extern float (*wkQTMovieMaxTimeSeekable)
>  extern int (*wkQTMovieGetType)(QTMovie* movie);
>  extern BOOL (*wkQTMovieHasClosedCaptions)(QTMovie* movie);
>  extern void (*wkQTMovieSetShowClosedCaptions)(QTMovie* movie, BOOL showClosedCaptions);
> +extern void (*wkQTMovieSelectPreferredAlternates)(QTMovie* movie);

The parameter name isn't needed here. You might as well get rid of the QTMovie parameter name on wkQTMovieGetType, wkQTMovieHasClosedCaptions, and wkQTMovieSetShowClosedCaptions while you are at it.


> Index: WebCore/platform/mac/WebCoreSystemInterface.mm
> ===================================================================
> --- WebCore/platform/mac/WebCoreSystemInterface.mm	(revision 56339)
> +++ WebCore/platform/mac/WebCoreSystemInterface.mm	(working copy)
> @@ -61,6 +61,7 @@ float (*wkQTMovieMaxTimeSeekable)(QTMovi
>  int (*wkQTMovieGetType)(QTMovie* movie);
>  BOOL (*wkQTMovieHasClosedCaptions)(QTMovie* movie);
>  void (*wkQTMovieSetShowClosedCaptions)(QTMovie* movie, BOOL showClosedCaptions);
> +void (*wkQTMovieSelectPreferredAlternates)(QTMovie* movie);

And here.
Comment 7 Eric Carlson 2010-03-26 08:45:12 PDT
rdar://7717504
Comment 8 Jer Noble 2010-03-26 09:29:57 PDT
Created attachment 51748 [details]
Patch
Comment 9 Eric Carlson 2010-03-26 09:33:20 PDT
Comment on attachment 51748 [details]
Patch

Thanks!
Comment 10 Jer Noble 2010-03-30 17:12:57 PDT
Created attachment 52104 [details]
Patch
Comment 11 Jer Noble 2010-03-30 17:14:01 PDT
Created attachment 52105 [details]
New libWebKitSystemInterfaceLeopard.a
Comment 12 Jer Noble 2010-03-30 17:15:26 PDT
Created attachment 52106 [details]
New libWebKitSystemInterfaceSnowLeopard.a
Comment 13 Jer Noble 2010-03-30 17:16:08 PDT
Created attachment 52107 [details]
New libWebKitSystemInterfaceTiger.a
Comment 14 Eric Seidel (no email) 2010-03-30 17:16:33 PDT
The commit queue will reject patches from contributers not listed as reviewers or committers in:
http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py
since I don't see your name there yet, I assume you didn't mean to mark these as r+/cq+?

I think the flags are mostly explained in:
http://webkit.org/coding/contributing.html
but I'm happy to answer questions about them if you have them. :)
Comment 15 WebKit Commit Bot 2010-03-30 17:17:02 PDT
Comment on attachment 52104 [details]
Patch

Rejecting patch 52104 from review queue.

jer.noble@apple.com does not have reviewer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/committers.py.

- If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have reviewer rights please correct the error in WebKitTools/Scripts/webkitpy/committers.py by adding yourself to the file (no review needed).  Due to bug 30084 the commit-queue will require a restart after your change.  Please contact eseidel@chromium.org to request a commit-queue restart.  After restart the commit-queue will correctly respect your reviewer rights.
Comment 16 Jer Noble 2010-03-30 17:17:38 PDT
Attempted to add the new libraries as part of the patch, but the resulting patch file was too large.  Attached the new libraries as binaries to this bug.
Comment 17 Jer Noble 2010-03-30 17:20:14 PDT
Sorry, those patches/libraries should have been marked as review?.
Comment 18 WebKit Review Bot 2010-03-30 17:24:06 PDT
Attachment 52105 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 19 WebKit Review Bot 2010-03-30 17:26:10 PDT
Attachment 52106 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 20 WebKit Review Bot 2010-03-30 17:27:20 PDT
Attachment 52107 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 Eric Seidel (no email) 2010-03-31 00:22:44 PDT
Sorry, looks like check-webkit-style doesn't understand binary attachments.  I've filed bug 36872.
Comment 22 Eric Seidel (no email) 2010-04-01 17:36:53 PDT
Comment on attachment 52105 [details]
New libWebKitSystemInterfaceLeopard.a

The commit-queue would barf if this were wrong, so it's safe to cq+ this blindly.
Comment 23 Eric Seidel (no email) 2010-04-01 17:37:18 PDT
Comment on attachment 52105 [details]
New libWebKitSystemInterfaceLeopard.a

However, we should probably wait until the main change is r+'d before landing.
Comment 24 Eric Carlson 2010-04-02 16:32:23 PDT
http://trac.webkit.org/changeset/57031