Last Comment Bug 725152 - Stereoscopic 3D videos not properly detected
: Stereoscopic 3D videos not properly detected
Status: RESOLVED FIXED
[qa+]
: regression
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: 11 Branch
: All All
: -- normal (vote)
: mozilla13
Assigned To: Joe Olivas
:
:
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
fixed


Attachments
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 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 http://www.youtube.com/watch?v=wNXCo6vEQRc&feature=html5_3d 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: http://hg.mozilla.org/mozilla-central/file/ede0d3cd7e3a/gfx/layers/d3d10/ImageLayerD3D10.cpp#l221


Expected results:

yuvImage->mData.mStereoMode should be returning the expected mode so the S3D path is taken.
Comment 1 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 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 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 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 Matthew Gregan [:kinetik] 2012-02-13 18:36:25 PST
http://hg.mozilla.org/integration/mozilla-inbound/rev/48d2243ca758
Comment 6 Marco Bonardo [::mak] 2012-02-15 08:43:27 PST
https://hg.mozilla.org/mozilla-central/rev/48d2243ca758
Comment 7 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 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 Matthew Gregan [:kinetik] 2012-03-16 13:28:29 PDT
mozilla-release try push:
https://tbpl.mozilla.org/?tree=Try&rev=f7621fb80ce5

Results should turn up here shortly:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mgregan@mozilla.com-f7621fb80ce5/
Comment 10 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 Matthew Gregan [:kinetik] 2012-03-19 16:50:54 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/6c62f6405499
Comment 12 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: http://www.3dvisionlive.com/3d_video. 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 Jason Smith [:jsmith] 2012-04-20 20:02:19 PDT
Results from Geo and I's testing

Prerequistes

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.

Testing

We tested the following different videos:

    http://www.3dvisionlive.com/3d_video Silverlight Oil Rush 3D Trailer
    http://www.3dvisionlive.com/3d_video Silverlight Oil Rush 3D Trailer Full Screen
    http://www.3dvisionlive.com/content/nvidia-3d-vision-pc-0 Embedded YouTube HTML 5 demo
    http://www.youtube.com/watch?v=T9fpI3IzV20 YouTube HTML 5 video
    http://www.youtube.com/watch?v=T9fpI3IzV20 YouTube HTML 5 video Full Screen

Results

    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.