725152 - Stereoscopic 3D videos not properly detected

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Joe Olivas]

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]

Attachment: 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]]

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]

Attachment: Check that mForceStereoMode is explicity set

Updating to the suggestions made above.

Comment 4

[Matthew Gregan [:kinetik]]

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]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/48d2243ca758

Comment 6

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/48d2243ca758

Comment 7

[Alex Keybl [:akeybl]]

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]]

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]]

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/[e-mail]-f7621fb80ce5/

Comment 10

[Alex Keybl [:akeybl]]

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]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6c62f6405499

Comment 12

[Vlad [QA]]

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]]

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.