makarandp0 Github contribution chart
makarandp0 Github Stats
makarandp0 Most Used Languages

Activity

27 Sep 2022

Issue Comment

Makarandp0

hyperFormula does not catch syntax error with dangling comma, and returns invalid result

Description

for a formula with syntax error (dangling comma) excel catches the error, but hyperformula doesn't

Steps to reproduce

import { HyperFormula } from 'hyperformula';

describe('hyperformula', () => {
  it('repro ', () => {
    const hfInstance = HyperFormula.buildFromArray([], {
      licenseKey: 'gpl-v3',
    });

    // this part works as expected
    const goodFormula = '=AND(1, 1)';
    const result1 = hfInstance.calculateFormula(goodFormula, 0);
    expect(result1).toStrictEqual(true);

    // hyperFormula did not catch the syntax error here.
    const badFormula = '=AND(1, 1,)';
    const result2 = hfInstance.calculateFormula(badFormula, 0);
    expect(result2).toStrictEqual(false);
  });
});


* HyperFormula version: 2.1.0
* Browser Name and version: N/A
* Operating System: N/A 

Forked On 27 Sep 2022 at 03:42:13

Makarandp0

Thank you for your response @sequba - You are of-course right, I see the same behavior in excel now for AND. I remember excel not allowing me to enter formula with dangling comma, but I must have been trying using some other function.

Thanks agains!

Commented On 27 Sep 2022 at 03:42:13
Issue Comment

Makarandp0

CRITICAL: noiseCancellationOptions overwrites mediaStreamTrack

  • [x] I have verified that the issue occurs with the latest twilio-video.js release and is not marked as a known issue in the CHANGELOG.md.
  • [x] I reviewed the Common Issues and open GitHub issues and verified that this report represents a potentially new issue.
  • [x] I verified that the Quickstart application works in my environment.
  • [x] I am not sharing any Personally Identifiable Information (PII) or sensitive account information (API keys, credentials, etc.) when reporting this issue.

Code to reproduce the issue:

const mediaRequest = await Video.createLocalTracks({
  audio: {
    noiseCancellationOptions: {
      sdkAssetsPath: `${location.origin}/${path}`,
      vendor: 'krisp',
    },
  },
}); 

Expected behavior:

Returned audiotrack should belong to navigator.mediaDevices.enumerateDevices()

Actual behavior: it's not possible to match returned track with navigator.mediaDevices.enumerateDevices() The new track belongs to the AudioDestinationNode and deviceId doesn't belongs to any enumerateDevices

Software versions:

  • [x] Browser(s): Chrome 104
  • [x] Operating System: MacOS Monterey
  • [x] twilio-video.js: 2.24.0
  • [x] Third-party libraries (e.g., Angular, React, etc.): Vue.js

Forked On 13 Sep 2022 at 04:25:46

Makarandp0

@vbabenko, noiseCancellation does overwrite the LocalAudioTrack.mediaStreamTrack, but it does store original track in LocalAudioTrack.noiseCancellation.srcTrack. Does that help in identifying the device used?

Commented On 13 Sep 2022 at 04:25:46

Makarandp0

changelog update for 2.23.1-rc2 (#1858)

Pushed On 17 Aug 2022 at 05:23:37

Makarandp0

changelog update for 2.23.1-rc2

Created On 17 Aug 2022 at 05:23:37

Makarandp0

changelog update for 2.23.-rc2

Created On 17 Aug 2022 at 04:46:14
Create Branch
Makarandp0 In twilio/twilio-video.js Create Branchprepare_for_2.21.1-rc2

Makarandp0

Twilio’s Programmable Video JavaScript SDK

On 17 Aug 2022 at 04:44:49

Makarandp0

adds getVersion and IsSupported checks for audio plugin (#1852)

  • version + issupported checks for audio plugin

  • krisp version 1.0.0

  • make test files small

  • move rnnoise files to test/assets

Pushed On 15 Aug 2022 at 04:03:06

Makarandp0

adds getVersion and IsSupported checks for audio plugin

Created On 15 Aug 2022 at 04:03:04
Issue Comment

Makarandp0

adds getVersion and IsSupported checks for audio plugin

Audio Plugins are now required to support getVersion and isSupported functions. Those will be used by SDK when loading the audio plugins. When version or isSupported mismatch is found SDK will not use noise cancellation plugin.

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • [ ] I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

Description

A description of what this PR does.

Burndown

Before review

  • [ ] Updated CHANGELOG.md if necessary
  • [ ] Added unit tests if necessary
  • [ ] Updated affected documentation
  • [ ] Verified locally with npm test
  • [ ] Manually sanity tested running locally
  • [ ] Included screenshot as PR comment (if needed)
  • [ ] Ready for review

Forked On 15 Aug 2022 at 04:02:27

Makarandp0

got +1 from @charliesantos offline: https://twilio.slack.com/archives/C01LMDCUF37/p1660579253767719?thread_ts=1660253627.569549&cid=C01LMDCUF37

Commented On 15 Aug 2022 at 04:02:27

Makarandp0

move rnnoise files to test/assets

Pushed On 11 Aug 2022 at 04:13:10
Issue Comment

Makarandp0

adds getVersion and IsSupported checks for audio plugin

Audio Plugins are now required to support getVersion and isSupported functions. Those will be used by SDK when loading the audio plugins. When version or isSupported mismatch is found SDK will not use noise cancellation plugin.

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • [ ] I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

Description

A description of what this PR does.

Burndown

Before review

  • [ ] Updated CHANGELOG.md if necessary
  • [ ] Added unit tests if necessary
  • [ ] Updated affected documentation
  • [ ] Verified locally with npm test
  • [ ] Manually sanity tested running locally
  • [ ] Included screenshot as PR comment (if needed)
  • [ ] Ready for review

Forked On 10 Aug 2022 at 09:21:26

Makarandp0

@makarandp0 how does this work if the sdk supports multiple versions?

@charliesantos, SDK will require that major version must match and minor version must be greater than supported minor version. As long as those parameters work, sdk can support multiple minor versions.

Commented On 10 Aug 2022 at 09:21:26

Makarandp0

make test files small

Pushed On 10 Aug 2022 at 08:25:51

Makarandp0

krisp version 1.0.0

Pushed On 10 Aug 2022 at 08:16:52

Makarandp0

adds getVersion and IsSupported checks for audio plugin

Created On 10 Aug 2022 at 08:12:33
Create Branch
Makarandp0 In twilio/twilio-video.js Create Branchkrisp_versioning

Makarandp0

Twilio’s Programmable Video JavaScript SDK

On 10 Aug 2022 at 08:10:01
Issue Comment

Makarandp0

Merge from master

This change merges from master, so that large room can use krisp noise cancelation beta. Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • [ ] I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

Description

A description of what this PR does.

Burndown

Before review

  • [ ] Updated CHANGELOG.md if necessary
  • [ ] Added unit tests if necessary
  • [ ] Updated affected documentation
  • [ ] Verified locally with npm test
  • [ ] Manually sanity tested running locally
  • [ ] Included screenshot as PR comment (if needed)
  • [ ] Ready for review

Forked On 09 Aug 2022 at 05:59:44

Makarandp0

Thank you for feedback @manjeshbhargav Please take another look

Commented On 09 Aug 2022 at 05:59:44

Makarandp0

incorporate feedback

Pushed On 09 Aug 2022 at 05:59:12

Makarandp0

Video 11047 src track was not stopped when stopping krisp enabled LocalAudioTrack (#1846)

  • stop srcTrack

  • changelog

Pushed On 04 Aug 2022 at 04:54:57

Makarandp0

Video 11047 src track was not stopped when stopping krisp enabled LocalAudioTrack

Created On 04 Aug 2022 at 04:54:56
Issue Comment

Makarandp0

Video 11047 src track was not stopped when stopping krisp enabled LocalAudioTrack

Fixed an issue where LocalAudioTrack.stop() was not stopping underlying noiseCancellation.srcTrack.
Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • [ ] I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

Description

A description of what this PR does.

Burndown

Before review

  • [ ] Updated CHANGELOG.md if necessary
  • [ ] Added unit tests if necessary
  • [ ] Updated affected documentation
  • [ ] Verified locally with npm test
  • [ ] Manually sanity tested running locally
  • [ ] Included screenshot as PR comment (if needed)
  • [ ] Ready for review

Forked On 03 Aug 2022 at 08:11:15

Makarandp0

Do we need to do something similar to .disable(), .restart() etc? Do these method affect noise cancellation in anyway when they are called?

@charliesantos, we do involve noiseCancellation for restart here but there isn't anything needed for disable.

Commented On 03 Aug 2022 at 08:11:15

Makarandp0

VIDEO-8756 Fix Room.getStats()

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • [x] I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

Description

This PR fixes the broken Room.getStats() behavior in large rooms, based on the proposal described here.

Highlights

  • No changes for LocalTrack stats, since they already work.
  • For each RemoteTrackPublication, we cache the following stats objects:
    • _switchOffStateChangeStats - the RemoteTrack stats captured at the most recent switch off state change
    • _switchOnMediaStreamTrackStats - the MediaStreamTrack stats captured when the RemoteTrack is switched on
  • Ensure that RoomV2.getStats() returns the stats for the switched on RemoteTracks.
  • RoomV3.getStats() will take the RemoteTrack stats returned by RoomV2.getStats() and perform the following transformations:
    • Add stats for switched off RemoteTracks by accessing the cached _switchOffStateChangeStats
    • Adjust stats for switched on RemoteTracks using the following rules:
      • If the stats property represents a cumulative value, like bytesReceived, then stats[property] = _switchOffStateChangeStats[property] + sampleStatsFromRoomV2getStats[property] - _switchOnMediaStreamTrackStats[property]
      • If the stats property represents a snapshot value, like jitter, then stats[property] = sampleStatsFromRoomV2getStats[property]

Burndown

Before review

  • [ ] Updated CHANGELOG.md if necessary
  • [ ] Added unit tests if necessary
  • [ ] Updated affected documentation
  • [ ] Verified locally with npm test
  • [x] Manually sanity tested running locally
  • [ ] Included screenshot as PR comment (if needed)
  • [x] Ready for review

Merged On 03 Aug 2022 at 07:11:09

Makarandp0

I have some minor suggestions, but looks very good

Commented On 03 Aug 2022 at 07:11:09

Makarandp0

VIDEO-8756 Fix Room.getStats()

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • [x] I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

Description

This PR fixes the broken Room.getStats() behavior in large rooms, based on the proposal described here.

Highlights

  • No changes for LocalTrack stats, since they already work.
  • For each RemoteTrackPublication, we cache the following stats objects:
    • _switchOffStateChangeStats - the RemoteTrack stats captured at the most recent switch off state change
    • _switchOnMediaStreamTrackStats - the MediaStreamTrack stats captured when the RemoteTrack is switched on
  • Ensure that RoomV2.getStats() returns the stats for the switched on RemoteTracks.
  • RoomV3.getStats() will take the RemoteTrack stats returned by RoomV2.getStats() and perform the following transformations:
    • Add stats for switched off RemoteTracks by accessing the cached _switchOffStateChangeStats
    • Adjust stats for switched on RemoteTracks using the following rules:
      • If the stats property represents a cumulative value, like bytesReceived, then stats[property] = _switchOffStateChangeStats[property] + sampleStatsFromRoomV2getStats[property] - _switchOnMediaStreamTrackStats[property]
      • If the stats property represents a snapshot value, like jitter, then stats[property] = sampleStatsFromRoomV2getStats[property]

Burndown

Before review

  • [ ] Updated CHANGELOG.md if necessary
  • [ ] Added unit tests if necessary
  • [ ] Updated affected documentation
  • [ ] Verified locally with npm test
  • [x] Manually sanity tested running locally
  • [ ] Included screenshot as PR comment (if needed)
  • [x] Ready for review

Forked On 03 Aug 2022 at 07:03:18

Makarandp0

this parameters keep increasing. Please change them to use object destructuring, this makes it much readable at caller site. So instead of ``` constructor( participantState, getPendingTrackReceiver, getInitialTrackSwitchOffState, setPriority, setRenderHint, clearTrackHint, getTrackStats, options ) ``` we should change to: ``` constructor({ participantState, getPendingTrackReceiver, getInitialTrackSwitchOffState, setPriority, setRenderHint, clearTrackHint, getTrackStats, options }) ```
On 03 Aug 2022 at 07:03:18

Makarandp0

VIDEO-8756 Fix Room.getStats()

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • [x] I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

Description

This PR fixes the broken Room.getStats() behavior in large rooms, based on the proposal described here.

Highlights

  • No changes for LocalTrack stats, since they already work.
  • For each RemoteTrackPublication, we cache the following stats objects:
    • _switchOffStateChangeStats - the RemoteTrack stats captured at the most recent switch off state change
    • _switchOnMediaStreamTrackStats - the MediaStreamTrack stats captured when the RemoteTrack is switched on
  • Ensure that RoomV2.getStats() returns the stats for the switched on RemoteTracks.
  • RoomV3.getStats() will take the RemoteTrack stats returned by RoomV2.getStats() and perform the following transformations:
    • Add stats for switched off RemoteTracks by accessing the cached _switchOffStateChangeStats
    • Adjust stats for switched on RemoteTracks using the following rules:
      • If the stats property represents a cumulative value, like bytesReceived, then stats[property] = _switchOffStateChangeStats[property] + sampleStatsFromRoomV2getStats[property] - _switchOnMediaStreamTrackStats[property]
      • If the stats property represents a snapshot value, like jitter, then stats[property] = sampleStatsFromRoomV2getStats[property]

Burndown

Before review

  • [ ] Updated CHANGELOG.md if necessary
  • [ ] Added unit tests if necessary
  • [ ] Updated affected documentation
  • [ ] Verified locally with npm test
  • [x] Manually sanity tested running locally
  • [ ] Included screenshot as PR comment (if needed)
  • [x] Ready for review

Forked On 03 Aug 2022 at 07:05:28

Makarandp0

```suggestion const cumulativeStatsProps = [ ```
On 03 Aug 2022 at 07:05:28

Makarandp0

VIDEO-8756 Fix Room.getStats()

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • [x] I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

Description

This PR fixes the broken Room.getStats() behavior in large rooms, based on the proposal described here.

Highlights

  • No changes for LocalTrack stats, since they already work.
  • For each RemoteTrackPublication, we cache the following stats objects:
    • _switchOffStateChangeStats - the RemoteTrack stats captured at the most recent switch off state change
    • _switchOnMediaStreamTrackStats - the MediaStreamTrack stats captured when the RemoteTrack is switched on
  • Ensure that RoomV2.getStats() returns the stats for the switched on RemoteTracks.
  • RoomV3.getStats() will take the RemoteTrack stats returned by RoomV2.getStats() and perform the following transformations:
    • Add stats for switched off RemoteTracks by accessing the cached _switchOffStateChangeStats
    • Adjust stats for switched on RemoteTracks using the following rules:
      • If the stats property represents a cumulative value, like bytesReceived, then stats[property] = _switchOffStateChangeStats[property] + sampleStatsFromRoomV2getStats[property] - _switchOnMediaStreamTrackStats[property]
      • If the stats property represents a snapshot value, like jitter, then stats[property] = sampleStatsFromRoomV2getStats[property]

Burndown

Before review

  • [ ] Updated CHANGELOG.md if necessary
  • [ ] Added unit tests if necessary
  • [ ] Updated affected documentation
  • [ ] Verified locally with npm test
  • [x] Manually sanity tested running locally
  • [ ] Included screenshot as PR comment (if needed)
  • [x] Ready for review

Merged On 03 Aug 2022 at 07:11:09

Makarandp0

I have some minor suggestions, but looks very good

Commented On 03 Aug 2022 at 07:11:09