-
-
Notifications
You must be signed in to change notification settings - Fork 7
feat(HikVision): support IDispose interface #783
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -237,6 +237,10 @@ export function stopRealPlay(id) { | |||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export function dispose(id) { | ||||||||||||||||||||||||||||||
| stopRealPlay(id); | ||||||||||||||||||||||||||||||
| logout(id); | ||||||||||||||||||||||||||||||
| WebVideoCtrl.I_DestroyPlugin(); | ||||||||||||||||||||||||||||||
|
Comment on lines
+240
to
+242
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
Comment on lines
+240
to
+243
|
||||||||||||||||||||||||||||||
| stopRealPlay(id); | |
| logout(id); | |
| WebVideoCtrl.I_DestroyPlugin(); | |
| try { | |
| stopRealPlay(id); | |
| } catch (e) { | |
| console.error('Error stopping real play:', e); | |
| } | |
| try { | |
| logout(id); | |
| } catch (e) { | |
| console.error('Error logging out:', e); | |
| } | |
| WebVideoCtrl.I_DestroyPlugin(); |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WebVideoCtrl.I_DestroyPlugin() appears to destroy the entire plugin instance globally, not just for a specific id. If multiple HikVision components can exist simultaneously (which is suggested by the id parameter pattern throughout the codebase), calling this in dispose(id) will destroy the plugin for all instances, not just the one being disposed.
Consider checking if there are any remaining instances before destroying the plugin:
Data.remove(id);
if (Data.size === 0) {
WebVideoCtrl.I_DestroyPlugin();
}Or, if the plugin is designed to be per-instance, ensure proper instance management in the WebVideoCtrl API.
| WebVideoCtrl.I_DestroyPlugin(); | |
| Data.remove(id); | |
| Data.remove(id); | |
| if (Data.size === 0) { | |
| WebVideoCtrl.I_DestroyPlugin(); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Destroying the global plugin in a per-id dispose may affect other active sessions.
If
WebVideoCtrlis a singleton shared across multipleids, callingI_DestroyPlugin()here will shut it down for all sessions whenever any oneidis disposed. That can break other active or subsequent sessions. You may need to only destroy the plugin when the lastidis disposed, or move plugin lifecycle management outside this per-iddisposepath.