Closed
Bug 584259
Opened 11 years ago
Closed 10 years ago
Add experimental support for packed format 3D video to libnestegg/WebMReader
Categories
(Core :: Audio/Video, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: kinetik, Assigned: kinetik)
References
Details
Attachments
(2 files, 11 obsolete files)
6.75 KB,
patch
|
cajbir
:
review+
roc
:
approval2.0+
|
Details | Diff | Splinter Review |
15.58 KB,
patch
|
Details | Diff | Splinter Review |
There's standardization work on the Matroska development list that will enable specify a bunch of different 3D video formats: http://lists.matroska.org/pipermail/matroska-devel/2010-July/003741.html And we have a patch from NVIDIA to add support for side-by-side packed 3D to the rendering code over in bug 584255. Once the Matroska side is nailed down, we should add experimental support to libnestegg/WebMReader to expose this metadata to the render code so it can enable 3D rendering when possible.
Comment 1•11 years ago
|
||
I hear that progress has been made on the spec side.
Assignee | ||
Comment 2•11 years ago
|
||
I think this is all that's required on the nestegg side. I won't push this to upstream nestegg until we've nailed down exactly what we need. I'm waiting to hear back from Alok about mapping these values to the ones wanted by the NVIDIA 3D API. For the decoder/playback engine, it looks like storing the stereo mode in nsVideoInfo and then creating the appropriate format Image from the ImageContainer is the right way to pass this data into the Layers rendering code.
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•11 years ago
|
||
Something like this.
Assignee | ||
Comment 4•11 years ago
|
||
nestegg patch imported into tree.
Attachment #487724 -
Attachment is obsolete: true
Assignee | ||
Comment 5•11 years ago
|
||
Updated with other modes. This is all completely untested, but it should work. Apply both patches, set gfx.3d_video.enabled to true, and play a video with the appropriate StereoMode value in the container.
Attachment #487738 -
Attachment is obsolete: true
Assignee | ||
Comment 7•11 years ago
|
||
Windows build fixes courtesy cpearce.
Attachment #487765 -
Attachment is obsolete: true
Assignee | ||
Comment 8•11 years ago
|
||
Another fix. With these patches, an appropriately generated video, the pref enabled, and Layers mode forced to D3D9, this gets as far as http://mxr.mozilla.org/mozilla-central/source/gfx/layers/d3d9/ImageLayerD3D9.cpp#192 but fails because m3DVStreaming is NULL due to http://mxr.mozilla.org/mozilla-central/source/gfx/layers/d3d9/Nv3DVUtils.cpp#86 failing. This probably means I don't have the appropriate drivers or hardware on the machine I tested on.
Attachment #487817 -
Attachment is obsolete: true
Assignee | ||
Comment 9•11 years ago
|
||
Another minor build fix.
Attachment #487828 -
Attachment is obsolete: true
Comment 10•11 years ago
|
||
Hi Matthew, I've verified that your patches work perfectly with a 3D WebM video and appropriate drivers and hardware.
Assignee | ||
Comment 11•11 years ago
|
||
Thanks for verifying! We'll need to get our hands on some testing equipment so we can work on this locally in the near future. A couple of notes: We can probably remove the gfx.3d_video.enabled pref and allow the code to rely on: 3D driver support being present and the appropriate StereoMode flag in the container. I also need to verify that the current behaviour wrt this note in the Matroska spec notes: The pixel count of the track (PixelWidth/PixelHeight) should be the raw amount of pixels (for example 3840x1080 for full HD side by side) and the DisplayWidth/Height in pixels should be the amount of pixels for one plane (1920x1080 for that full HD stream). ...because I have a feeling that the current code will do the wrong thing in this case. It looks like DisplayWidth/Height needs to be special cased for files with StereoMode set. :-( Test Win32 builds of the current patches are here (until they expire in 2 weeks): http://stage.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/mgregan@mozilla.com-3d09ea745f0a/tryserver-win32/ Also, here's a link to the current WebM spec discussion: http://groups.google.com/a/webmproject.org/group/webm-discuss/browse_thread/thread/94c71028b688b998/561ed3d008d19568#561ed3d008d19568
Comment 12•11 years ago
|
||
"It looks like DisplayWidth/Height needs to be special cased for files with StereoMode set." No, the example you mentioned is when the video is not expanded. In the more common (current use in IPTV for example) each side is shrinked horizontally to fit a normal frame to decode. So the pixel values are 1920/1080 and the display values are also 1920/1080. Knowing that it's horizontal side by side you know you have to cut in the middle. And then the display values tell you how these rectangles are supposed to be displayed.
Assignee | ||
Comment 13•10 years ago
|
||
Adds STEREO_MODE_RIGHT_LEFT, which was added to the Matroska spec recently.
Attachment #487763 -
Attachment is obsolete: true
Assignee | ||
Comment 14•10 years ago
|
||
Adds STEREO_MODE_RIGHT_LEFT. Removes the gfx.3d_video.enabled pref completely and relies on the stereo mode being set on the image. Adds a pref which is read during metadata load in nsWebMReader that allows a particular stereo mode to be forced for testing files that don't have a StereoMode element. This is a temporary measure until the tools (and YouTube) have been modified to support adding this element. The pref is an integer pref called "media.webm.force_stereo_mode", with the following valid values: 1 = LEFT_RIGHT 2 = RIGHT_LEFT 3 = TOP_BOTTOM 4 = BOTTOM_TOP anything else = MONO As with the earlier patches, you also need to force D3D9 layers mode (if you default to D3D10) due to only having support for stereo video in the D3D9 code. Bug 617220 covers adding support to D3D10.
Attachment #488150 -
Attachment is obsolete: true
Assignee | ||
Comment 15•10 years ago
|
||
Win32 try builds will turn up at http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds /mgregan@mozilla.com-e532ff85bd45
Assignee | ||
Comment 16•10 years ago
|
||
Don't override the StereoMode value if the pref is not set.
Attachment #496037 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #496033 -
Flags: review?(chris.double)
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 496374 [details] [diff] [review] patch v1.6 Bas might be better for the D3D9 review, but I picked Robert because he can review both modules.
Attachment #496374 -
Flags: review?(roc)
The StereoMode enum could move from PlanarYCbCrImage::Data to the mozilla::layers namespace. That would make the code more concise, and at some point we'll probably want to reuse it for other image types. nsVideoInfo: + enum StereoMode { + STEREO_MODE_MONO, + STEREO_MODE_LEFT_RIGHT, + STEREO_MODE_RIGHT_LEFT, + STEREO_MODE_BOTTOM_TOP, + STEREO_MODE_TOP_BOTTOM + }; We can just use the mozilla::layers::StereoMode instead. Otherwise looks good.
Assignee | ||
Comment 19•10 years ago
|
||
With roc's changes. Also prefixed the Nv3DUtils stereo mode stuff with Nv_.
Attachment #496374 -
Attachment is obsolete: true
Attachment #496395 -
Flags: review?(roc)
Attachment #496374 -
Flags: review?(roc)
Comment on attachment 496395 [details] [diff] [review] patch v1.7 Drop a 'using namespace mozilla::layers;" into nsWebMReader.cpp and lose those ugly prefixes
Attachment #496395 -
Flags: review?(roc)
Attachment #496395 -
Flags: review+
Attachment #496395 -
Flags: approval2.0+
Assignee | ||
Comment 21•10 years ago
|
||
Final changes. Carrying forward r+ and a+.
Attachment #496395 -
Attachment is obsolete: true
Comment 22•10 years ago
|
||
The latest build works perfect w/ associated drivers and hardware. Thx.
Updated•10 years ago
|
Attachment #496033 -
Flags: review?(chris.double) → review+
Assignee | ||
Updated•10 years ago
|
Attachment #496033 -
Flags: approval2.0?
Attachment #496033 -
Flags: approval2.0? → approval2.0+
Assignee | ||
Comment 23•10 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/28f582d9b6d8 http://hg.mozilla.org/mozilla-central/rev/df3122f03328
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 24•10 years ago
|
||
Can someone with the correct hardware/drivers please mark this verified?
Assignee | ||
Comment 25•10 years ago
|
||
We've got a test rig in the NZ office now, I verified that this works in a nightly yesterday.
Status: RESOLVED → VERIFIED
Comment 26•10 years ago
|
||
Same here.
Depends on: 714445
You need to log in
before you can comment on or make changes to this bug.
Description
•