Enable VP9 SVC with webrtc m94#131
Conversation
|
|
||
| offer = this->pc->CreateOffer(options); | ||
| auto localSdpObject = sdptransform::parse(offer); | ||
| json localSdpObject = sdptransform::parse(offer); |
There was a problem hiding this comment.
Nothing, just my personal habit. You can ignore it
There was a problem hiding this comment.
I cannot ignore it, it's part of your proposed PR 😀
There was a problem hiding this comment.
I'll pay attention next time 🙂
There was a problem hiding this comment.
yes please, move it back to auto.
| { | ||
| // Panic here. Try to undo things. | ||
| transceiver->SetDirection(webrtc::RtpTransceiverDirection::kInactive); | ||
| //transceiver->SetDirection(webrtc::RtpTransceiverDirection::kInactive); |
There was a problem hiding this comment.
Why is this commented? If it doesn't make sense let's just remove the line. But why?
There was a problem hiding this comment.
just mark it,maybe i will change lower version of webrtc
There was a problem hiding this comment.
I don't understand. How does lower version of webrtc affect here? My question is: why is that line commented in your PR?
There was a problem hiding this comment.
I'm careless. The old interface has been deprecated：
RTC_DEPRECATED virtual void SetDirection(
RtpTransceiverDirection new_direction);
|
|
||
| return ( | ||
| jsonAttributeIt->get<std::string>() == "cname" && jsonIdIt->get<uint32_t>() == firstSsrc); | ||
| jsonAttributeIt->get<std::string>() == "cname" /*&& jsonIdIt->get<uint32_t>() == firstSsrc*/); |
There was a problem hiding this comment.
I refer to project mediasoup-client,
`const ssrcCnameLine = offerMediaObject.ssrcs
.find((line: any) => line.attribute === 'cname');
if (!ssrcCnameLine)
throw new Error('a=ssrc line with cname information not found');`
There was a problem hiding this comment.
Ok, then please remove the commented code. @jmillan do you know why we were doing this different here than in mediasoup-client? It doesn't make any sense to me.
|
@jmillan anything else from your side? should we merge and release? |
|
@harvestsure I'd appreciate a PR in the mediasoup-website project with updated API documentation. |
It looks good to me. I'Il merge it and release tomorrow if you don't do it before. |
|
Sorry for the delay, terribly busy. Will check this in next days but cannot provide with a proper ETA yet. |
|
@jmillan ping |
|
I'm testing this PR with m89. |
|
Actually m89 fails to compile with Xcode 13 and it's already very old. I'll try the newest stable branch instead. |
|
You didn't modify the tests, which do not compile. Please, adapt the test files accordingly and also, let me ask you to target this PR against the new libmediasoupclient which uses M94. |
harvestsure
changed the title
I have compiled using the M94 version with test files. |
|
There's a warning here @harvestsure,
Also, since this changes the API we'd appreciate a PR in the mediasoup-website project with updated API documentation |
I'll deal with it later |
|
Thanks, the doc will be updated. |
* Enable VP9 SVC with webrtc m89 webrtc::RtpEncodingParameters.scalability_mode Co-authored-by: yangcj <gupar@qq.com>
* Enable VP9 SVC with webrtc m89 webrtc::RtpEncodingParameters.scalability_mode Co-authored-by: yangcj <gupar@qq.com>
3 participants
Enable VP9 SVC with webrtc m94 webrtc::RtpEncodingParameters.scalability_mode