Skip to content

fix(plugin-meetings): remove JMP deduplication logic#4773

Merged
edvujic merged 3 commits intowebex:nextfrom
edvujic:jmp-deduplication-logic
Mar 25, 2026
Merged

fix(plugin-meetings): remove JMP deduplication logic#4773
edvujic merged 3 commits intowebex:nextfrom
edvujic:jmp-deduplication-logic

Conversation

@edvujic
Copy link
Copy Markdown
Contributor

@edvujic edvujic commented Mar 13, 2026

COMPLETES SPARK-786722

This pull request addresses

JMP deduplication logic was temporarily implemented in MediaRequestManager as a workaround. This responsibility has since been moved to the @webex/json-multistream layer, making the local deduplication code redundant.

by making the following changes

  • Removed previousStreamRequests property from MediaRequestManager
  • Removed checkIsNewRequestsEqualToPrev() method
  • Removed isEqual() method
  • Removed clearPreviousRequests() method
  • Simplified sendRequests() to always forward requests to the callback without local dedup checks
  • Removed unused isEmpty import from lodash
  • Removed clearPreviousRequests() call from ReconnectionManager
  • Updated unit tests in mediaRequestManager.ts and reconnection-manager/index.js to reflect the removal

Change Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Tooling change
  • Internal code refactor

The following scenarios were tested

  • Ran full unit test suite for @webex/plugin-meetings (yarn test:unit) — 3091 passing, 0 failing
  • Verified MediaRequestManager tests covering request creation, cancellation, commit, reset, trimming, and degradation still pass
  • Verified ReconnectionManager tests for multistream reconnection still pass after removing clearPreviousRequests assertions

The GAI Coding Policy And Copyright Annotation Best Practices

  • GAI was not used (or, no additional notation is required)
  • Code was generated entirely by GAI
  • GAI was used to create a draft that was subsequently customized or modified
  • Coder created a draft manually that was non-substantively modified by GAI (e.g., refactoring was performed by GAI on manually written code)
  • Tool used for AI assistance (GitHub Copilot / Other - specify)
    • Github Copilot
    • Other - Cursor
  • This PR is related to
    • Feature
    • Defect fix
    • Tech Debt
    • Automation

I certified that

  • I have read and followed contributing guidelines
  • I discussed changes with code owners prior to submitting this pull request
  • I have not skipped any automated checks
  • All existing and new tests passed
  • I have updated the documentation accordingly

Make sure to have followed the contributing guidelines before submitting.

@edvujic edvujic self-assigned this Mar 13, 2026
@edvujic edvujic added the validated If the pull request is validated for automation. label Mar 13, 2026
@edvujic edvujic marked this pull request as ready for review March 13, 2026 13:48
@edvujic edvujic requested review from a team as code owners March 13, 2026 13:48
@edvujic edvujic enabled auto-merge (squash) March 13, 2026 13:51
@k-wasniowski
Copy link
Copy Markdown
Contributor

k-wasniowski commented Mar 13, 2026

@edvujic Were you able to replicate the issue, and verify that it doesnt occur anymore?

@edvujic
Copy link
Copy Markdown
Contributor Author

edvujic commented Mar 18, 2026

@edvujic Were you able to replicate the issue, and verify that it doesnt occur anymore?

Yes, I verified this by joining a multistream meeting and having a user leave and rejoin. From the logs, the SDK no longer does any client-side dedup filtering, every commit() results in "media requests sent" and the JMP is actively catching and suppressing duplicates, logging Duplicate MediaRequest detected and will not be sent when it does so, and streams recovered correctly after the user rejoined. Let me know if you want to discuss further offline.

@edvujic edvujic requested a review from antsukanova March 18, 2026 11:10
chatgpt-codex-connector[bot]
chatgpt-codex-connector bot reviewed Mar 18, 2026
View reviewed changes
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 07980e4ca4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

rarajes2
rarajes2 approved these changes Mar 20, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@rarajes2 rarajes2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving with a few questions.

It would be nice to have a vidcast showcasing a sanity test on the meetings feature where multiple requests causing duplications are handled.

packages/@webex/plugin-meetings/src/multistream/mediaRequestManager.ts
`multistream:sendRequests --> detected duplicate WCME requests, skipping them... `
);
}
this.sendMediaRequestsCallback(streamRequests);
Copy link
Copy Markdown
Contributor

@rarajes2 rarajes2 Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q1: Is this the method which will handle the duplicate requests at @webex/json-multistream layer ?
Q2: The log multistream:sendRequests --> media requests sent. will now always get printed. Might be bit confusing. Maybe this.sendMediaRequestsCallback method can return some boolean and based on that we can say whether the request was actually sent or was discarded due to duplication.

Copy link
Copy Markdown
Contributor Author

@edvujic edvujic Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The dedup happens inside JmpSession.sendRequests() in the @webex/web-client-media-engine package, not @webex/json-multistream directly. It compares each incoming request array against this.lastSentMediaRequest using areStreamRequestsEqual, which does a comparison of policy, policy-specific info, stream IDs, max payload bitrate, and codec info. If identical, the request is dropped with a log at the WCME level.

  2. Fair enough... though I think making sendMediaRequestsCallback return a boolean is not practical here. The requestMedia() method in WCME is void And even internally, the actual dedup + send may be deferred (queued into pendingJmpTasks if the data channel isn't open yet).

@edvujic edvujic merged commit 32d4f9c into webex:next Mar 25, 2026
11 checks passed
fnowakow pushed a commit to fnowakow/webex-js-sdk that referenced this pull request Mar 25, 2026
@edvujic @fnowakow
fix(plugin-meetings): remove JMP deduplication logic (webex#4773)
c072578 
Co-authored-by: evujici <evujici@cisco.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

@antsukanova antsukanova antsukanova approved these changes

@k-wasniowski k-wasniowski k-wasniowski approved these changes

@rarajes2 rarajes2 rarajes2 approved these changes

@szotrabh szotrabh Awaiting requested review from szotrabh

@SomeBody16 SomeBody16 Awaiting requested review from SomeBody16

+1 more reviewer

@fnowakow fnowakow fnowakow approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@edvujic edvujic

Labels

validated If the pull request is validated for automation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@edvujic @k-wasniowski @antsukanova @rarajes2 @fnowakow