Last Comment Bug 725152 - Stereoscopic 3D videos not properly detected
: Stereoscopic 3D videos not properly detected
: regression
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: 11 Branch
: All All
-- normal (vote)
: mozilla13
Assigned To: Joe Olivas
: Maire Reavy [:mreavy] Please needinfo me
Depends on:
Blocks: 714445
  Show dependency treegraph
Reported: 2012-02-07 16:36 PST by Joe Olivas
Modified: 2012-04-20 20:02 PDT (History)
10 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patching to check mForceStereoMode as non-zero (1.66 KB, patch)
2012-02-13 15:39 PST, Joe Olivas
no flags Details | Diff | Splinter Review
Check that mForceStereoMode is explicity set (2.88 KB, patch)
2012-02-13 17:43 PST, Joe Olivas
kinetik: review+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description User image Joe Olivas 2012-02-07 16:36:28 PST
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120206 Firefox/13.0a1
Build ID: 20120207170854

Steps to reproduce:

Visiting some of the S3D videos on YouTube such as aren't being detected as S3D.

Actual results:

in gfx/layers/d3d10/ImageLayerD3DD10.cpp, checking the stereo mode with yuvImage->mData.mStereoMode is returning STEREO_MODE_MONO so the S3D path is being passed over. This is being done here:

Expected results:

yuvImage->mData.mStereoMode should be returning the expected mode so the S3D path is taken.
Comment 1 User image Joe Olivas 2012-02-13 15:39:17 PST
Created attachment 596823 [details] [diff] [review]
Patching to check  mForceStereoMode as non-zero

It appears that the stereo mode is getting properly set in the "nestegg" section, but is immediately overriden by the switch on mForceStereoMode, which is zero by default.

It appears the intent of zero means "unset", but it is going through the switch statement to the default: case, and getting set to STEREO_MODE_MONO.
Comment 2 User image Matthew Gregan [:kinetik] 2012-02-13 17:04:41 PST
Thanks for tracking this down!  It's a regression from bug 714445.  Before the change in that bug the value of forceStereoMode was only used if the preference had been set explicitly (which it isn't by default).

My preference is to completely remove the forceStereoMode machinery completely.  It's global for all videos (but only takes effect on load), which is confusing behaviour.  It was only added as a quick hack to allow testing before YouTube and WebM tools were able to add the StereoMode element to the container.

If we're going to keep forceStereoMode around for now, I'd prefer to retain the old behaviour.  To do that, add a new bool to enable forcing stereo mode, then change the GetInt call to:

mStereoModeForced = NS_SUCCEEDED(Preferences::GetInt("media.webm.force_stereo_mode", &mForceStereoMode);

...and wrap the switch in ReadMetadata inside a test for mStereoModeForced.
Comment 3 User image Joe Olivas 2012-02-13 17:43:01 PST
Created attachment 596875 [details] [diff] [review]
Check that mForceStereoMode is explicity set

Updating to the suggestions made above.
Comment 4 User image Matthew Gregan [:kinetik] 2012-02-13 17:52:10 PST
Comment on attachment 596875 [details] [diff] [review]
Check that mForceStereoMode is explicity set

Review of attachment 596875 [details] [diff] [review]:

Thanks!  I'll fix up the spacing nits and commit the patch shortly.

::: content/media/webm/nsWebMReader.cpp
@@ +313,5 @@
>          break;
>        }
> +      //Switch only when stereo mode is explicitly set 
> +      if(mStereoModeForced) {

Minor nit: spaces after // and if.

::: content/media/webm/nsWebMReader.h
@@ +244,5 @@
>    // Value of the "media.webm.force_stereo_mode" pref, which we need off the
>    // main thread.
>    PRInt32 mForceStereoMode;
> +
> +  //Boolean which is set to true when the "media.webm.force_stereo_mode" is

And here too.
Comment 5 User image Matthew Gregan [:kinetik] 2012-02-13 18:36:25 PST
Comment 6 User image Marco Bonardo [::mak] 2012-02-15 08:43:27 PST
Comment 7 User image Alex Keybl [:akeybl] 2012-03-16 09:59:15 PDT
In email, I've asked Matthew to kick off a try build based on m-r with this patch integrated in for testing in case we decide to include the fix in a FF11 chemspill. I've also requested that we nominate the patch for uplift to Beta 12 if deemed sufficiently low risk.
Comment 8 User image Matthew Gregan [:kinetik] 2012-03-16 12:54:31 PDT
Comment on attachment 596875 [details] [diff] [review]
Check that mForceStereoMode is explicity set

[Approval Request Comment]
Regression caused by (bug #): 714445
User impact if declined: stereoscopic 3d video is disabled
Testing completed (on m-c, etc.): fix has been in m-c for a month
Risk to taking this patch (and alternatives if risky): extremely low
String changes made by this patch: none
Comment 9 User image Matthew Gregan [:kinetik] 2012-03-16 13:28:29 PDT
mozilla-release try push:

Results should turn up here shortly:
Comment 10 User image Alex Keybl [:akeybl] 2012-03-19 09:18:28 PDT
Comment on attachment 596875 [details] [diff] [review]
Check that mForceStereoMode is explicity set

[Triage Comment]
Approved for Beta 12 since this is a recent regression and considered very low risk.
Comment 11 User image Matthew Gregan [:kinetik] 2012-03-19 16:50:54 PDT
Comment 12 User image Vlad [QA] 2012-04-18 05:22:02 PDT
Hi guys.
I have tried this on Firefox 12 beta 4 on Win 7 x 64 and the issue is still reproducing, meaning that the stereoscopic 3d videos in youtube are not working.

I've verified this using the Nvidia 3d Vision 2 wireless glasses with a Asus VG278H 3d monitor with a 3d built-in emitter.

The only videos that I was able to view in 3d stereoscopic were the ones from the Nvidia page: The nvidia player has an option which starts my emitter and thus activate the glasses. 
I didn't manage to make this work in youtube. The monitor emitter doesn't start at all.
Comment 13 User image Jason Smith [:jsmith] 2012-04-20 20:02:19 PDT
Results from Geo and I's testing


We confirmed hardware with NVIDIA's drivers. We also tested the demos NVIDIA had to make sure that we had 3D video worked as expected. We confirmed we had 3D video setup correctly.


We tested the following different videos: Silverlight Oil Rush 3D Trailer Silverlight Oil Rush 3D Trailer Full Screen Embedded YouTube HTML 5 demo YouTube HTML 5 video YouTube HTML 5 video Full Screen


    9.0.1 release
    * Entering/Exiting Silverlight video (partial and full screen) worked as expected (glasses on and off at right time)
    * Entering YouTube HTML 5 video worked as expected in all cases.
    * Exiting YouTube HTML 5 video by closing tab did not turn off glasses in any case. Exiting Firefox did.
    12.0 b6
    * Worked exactly the same as 9.0.1 release
    13.0 a2
    * Worked as 9.0.1 release, EXCEPT:
    * Entering YouTube HTML 5 full-screen video showed side-by-side rather than through alternating shutters.

Note You need to log in before you can comment on or make changes to this bug.