Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sink code work again #208

Merged
merged 5 commits into from Mar 2, 2019

Conversation

Projects
None yet
3 participants
@Manishearth
Copy link
Member

Manishearth commented Feb 21, 2019

fixes #191

@Manishearth Manishearth force-pushed the Manishearth:webrtc-fixes branch from 37ca016 to 705c8db Feb 25, 2019

@Manishearth Manishearth changed the title [WIP] Make sink code work again Make sink code work again Feb 25, 2019

@Manishearth Manishearth force-pushed the Manishearth:webrtc-fixes branch from 705c8db to d80fce4 Feb 25, 2019

@Manishearth

This comment has been minimized.

Copy link
Member Author

Manishearth commented Feb 25, 2019

Ready for review

r? @jdm

@Manishearth

This comment has been minimized.

Copy link
Member Author

Manishearth commented Feb 25, 2019

The issues were:

  • we shouldn't be vp8/opus encoding stuff going to the auto sinks
  • pipeline play state in webrtc was incorrect (and later: pipeline play state in my sink code was also broken)
  • decodebin was connected directly to the webrtc element instead of to the dynamically produced pad
  • the vaapi thing is broken, so gstreamer1.0-vaapi shouldn't be installed
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 25, 2019

☔️ The latest upstream changes (presumably #211) made this pull request unmergeable. Please resolve the merge conflicts.

@Manishearth Manishearth force-pushed the Manishearth:webrtc-fixes branch from d80fce4 to 3f29ecb Feb 25, 2019

@Manishearth Manishearth referenced this pull request Feb 27, 2019

Merged

Fix sink pad ordering #213

@Manishearth Manishearth force-pushed the Manishearth:webrtc-fixes branch from 3f29ecb to 94d0b66 Mar 2, 2019

@Manishearth

This comment has been minimized.

Copy link
Member Author

Manishearth commented Mar 2, 2019

Addressed.

Looks like requesting sink_1 also works, it probably had a bad interaction with some prior code.

I still have to prerequest pads, though.

@jdm

This comment has been minimized.

Copy link
Member

jdm commented Mar 2, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Mar 2, 2019

📌 Commit 94d0b66 has been approved by jdm

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Mar 2, 2019

⌛️ Testing commit 94d0b66 with merge 30bc863...

bors-servo added a commit that referenced this pull request Mar 2, 2019

Auto merge of #208 - Manishearth:webrtc-fixes, r=jdm
Make sink code work again

fixes #191
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Mar 2, 2019

☀️ Test successful - checks-travis
Approved by: jdm
Pushing 30bc863 to master...

@bors-servo bors-servo merged commit 94d0b66 into servo:master Mar 2, 2019

1 check passed

homu Test successful
Details

bors-servo added a commit that referenced this pull request Mar 2, 2019

Auto merge of #213 - Manishearth:sink-pad-ordering, r=jdm
Fix sink pad ordering

Fixes #210

Based on #208

This implements a version of the fix detailed in #210 (comment)

Instead of unlinking/relinking, we simply keep new streams unlinked until we decide to trigger an offer, at which point we link them to new request pads. The answerer side keeps them unlinked until it gets a remote offer, at which point it also requests pads and links them to the correctly numbered request pads.

A missing piece was that the payload number also has to match, so we keep track of that too.

r? @jdm @slomo (last two commits only)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.