Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.
/ kms-core Public archive

fix composite: kmsenctreebin.c use max-size-time instead of max-size-buffers #9

Merged
merged 1 commit into from
Jan 9, 2018

Conversation

ruddell
Copy link
Contributor

@ruddell ruddell commented Dec 5, 2017

Fix Kurento/bugtracker#197

I'm not sure of the original use-case of #7 (related mailing-list post), so instead of reverting the queue restriction, I changed it from max-size-buffers to max-size-time (also used in agnosticbin). The composite works as expected again (no audio glitches).

@kc7bfi Does this still change still apply to your issue?

Follows: #7

@jenkinskurento
Copy link

Hi there. Thanks for your PR.

I'm waiting for a Kurento member to verify that this patch is reasonable to test. If it is, they should reply with check out please on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

@jenkinskurento
Copy link

There were no errors, go have a cup of coffee...

@j1elo
Copy link
Member

j1elo commented Dec 21, 2017

If @kc7bfi doesn't comment against it, in the next days I'll merge this PR as we've tested that it actually solves the issue

@j1elo j1elo merged commit 42c657c into Kurento:master Jan 9, 2018
@kylefoley
Copy link

Do you know when this will be in a new release of KMS Core?

@j1elo
Copy link
Member

j1elo commented Jan 13, 2018

Our intention was to release the final 6.7.0 version in the first weeks of January, so we're already a bit late in our schedules. Some bug fixing in other areas have delayed the whole process.
I hope that we'll be able to publish the new version sometime in January

@ruddell ruddell deleted the fix-queue-filter branch January 13, 2018 15:53
@kylefoley
Copy link

I'm able to replicate this issue with kms-core 6.7.0. Can anyone else confirm?

@wnyaonjr
Copy link

hello, we are able to replicate with kms-core 6.7.0 as well.
any update on target timeline when to release the fix for this problem in MCU?
Thank you.

@ruddell
Copy link
Contributor Author

ruddell commented Feb 20, 2018

@wnyaonjr We discussed the issue occurring in 6.7.0 here: Kurento/bugtracker#228 We figured out that the "nightly" version in the repository is older than this commit and it is actually fixed. There are instructions at the bottom for installing the latest release which should be fixed.

If you can still reproduce and you are sure your version of kms-core has this commit, please open a bugtracker issue with all of the details.

@wnyaonjr
Copy link

thank you @ruddell, highly appreciate your swift response. we will re-test the latest version.

@j1elo
Copy link
Member

j1elo commented Jun 11, 2018

Hi @ruddell the 600ms seems to be causing audio synchronization issues.
Did you choose that number exclusively based on the same value in agnosticbin? Or there was some additional testing that validated this value (further than simply checking that it works)?

@ruddell
Copy link
Contributor Author

ruddell commented Jun 11, 2018

@j1elo Yes I chose it based on that value, I didn't do any technical tests other than making sure the elements still work (specifically WebRTC, Composite, RTPEndpoint, and PlayerEndpoint). I haven't noticed any issues running this but I'm running Kurento on a powerful server. If it causes issues, please revert but note that will break Composite element's audio (which was caused by #7).

prlanzarin added a commit to prlanzarin/kms-core that referenced this pull request Nov 11, 2019
Moved extra rtcp-fb add on answer to no-mid only SDPs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Composite Hub making audio choppy. KMS 6.6.3+
5 participants