Closed Bug 1081409 Opened 10 years ago Closed 10 years ago

mozCaptureStream video over peerConnection almost works (wrong length)

Categories

(Core :: WebRTC: Audio/Video, defect)

32 Branch
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox34 --- wontfix
firefox35 --- wontfix
firefox36 --- fixed

People

(Reporter: jib, Assigned: jesup)

References

()

Details

Attachments

(3 files, 3 obsolete files)

With the placeholder-track patch from Bug 1070127 and the attached workaround-patch, I'm able to run the URL test script (coming next) which sends a mozCaptureStream'ed video-file over peerConnection.

Without the workaround-patch it doesn't play and triggers an assert in MediaPipeline.cpp instead:

  MOZ_ASSERT(length == I420SIZE(width, height));

Removing the assert isn't enough; The frame length has to be correct (852*480 PLANAR_YCBCR - should be 613440, was 613480) See workaround-patch for details.
Attached video testfile.webm (obsolete) —
Test script doesn't work cross-origin so I'm uploading test file here.
Filed Bug 1081819 for the audio part.
relax the tests on length since TextureClients may add other fields/structures to the buffer the YUV data is in
Attachment #8503503 - Attachment is obsolete: true
Attachment #8504796 - Flags: review?(jib)
we'll also want to write a mochitest for forwarding video
Whiteboard: [webrtc-uplift]
Comment on attachment 8504796 [details] [diff] [review]
Fix forwarding of captured MediaStreams to PeerConnections

Review of attachment 8504796 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with comment fix.

::: media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
@@ +1219,1 @@
>      // XXX Consider making this a non-debug-only check if we ever implement

Patch makes this comment obsolete.
Attachment #8504796 - Flags: review?(jib) → review+
Attachment #8503505 - Attachment is obsolete: true
The oddest thing. Works in browser but not in mochitest harness. Timing?

Test is doctored to work in browser and use already uploaded file, though I've tested that the files don't make a difference.
Comment on attachment 8505245 [details]
test_peerConnection_capturedVideo.html - works in browser

Removing since it had .js dependencies that made it not run in bugzilla. Anyways, if you put this file ontop of the next patch, copy the dir to a web server and run it in a browser it works (whereas the mochitest does not for some reason).
Attachment #8505245 - Attachment is obsolete: true
Here's the mochitest I added but can't get to succeed in the mochitest harness, even though the same test running in a browser seems to. Ideas as to why are welcome.
Make sure it's adding the tracks to the right peerconnection....  

Also, I notice there's a "Skipping GUM" line; that means attachMedia() isn't called (which likely is fine and correct, but I just want to make sure skipping it didn't leave any landmines or unfinished items).
The "Skipping GUM" is intentional. I didn't need to remove that step as I found it's a no-op when I omit attachMedia. I'm addTracking manually to pcLocal._pc.

Regardless, that this test works in the browser (comment 8) I hope rules out most pilot errors.
Comment on attachment 8504796 [details] [diff] [review]
Fix forwarding of captured MediaStreams to PeerConnections

Review of attachment 8504796 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
@@ +1197,4 @@
>      uint8_t *y = data->mYChannel;
>  #ifdef DEBUG
>      uint8_t *cb = data->mCbChannel;
>      uint8_t *cr = data->mCrChannel;

Ugh should have caught that on review. cb and cr need to be defined all the time now, not just in debug.
(In reply to Randell Jesup [:jesup] from comment #12)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/a52b2b37e42a
> We can land the tests on a follow-up bug

I agree. The problems we're with the tests are surely unrelated to this patch.
https://hg.mozilla.org/mozilla-central/rev/3bfafff4ff07
Assignee: nobody → rjesup
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Blocks: 1097224
Whiteboard: [webrtc-uplift]
You need to log in before you can comment on or make changes to this bug.