feat(HikVision): add CapturePicture method #869
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideAdds a new CapturePicture API that operates with filenames and boolean success flags instead of JS stream references, and improves state tracking for sound and recording on the HikVision web plugin and its JavaScript side. Sequence diagram for new CapturePicture workflowsequenceDiagram
actor User
participant BlazorComponent
participant HikVisionWebPlugin
participant IJSRuntime
participant hikvision_js
participant WebVideoCtrl
User ->> BlazorComponent: Trigger capture
BlazorComponent ->> HikVisionWebPlugin: CapturePicture(fileName, token)
HikVisionWebPlugin ->> HikVisionWebPlugin: Check IsLogin && IsRealPlaying
alt Logged in and real playing
HikVisionWebPlugin ->> IJSRuntime: InvokeAsync<bool>("capturePicture", token, Id, fileName)
IJSRuntime ->> hikvision_js: capturePicture(id, szFileName)
hikvision_js ->> hikvision_js: if !szFileName then default name
hikvision_js ->> WebVideoCtrl: I_CapturePic(szFileName)
WebVideoCtrl -->> hikvision_js: success or failure
hikvision_js -->> IJSRuntime: true/false
IJSRuntime -->> HikVisionWebPlugin: true/false
HikVisionWebPlugin -->> BlazorComponent: bool success
else Not logged in or not playing
HikVisionWebPlugin -->> BlazorComponent: false
end
Class diagram for updated HikVisionWebPlugin capture and state APIsclassDiagram
class HikVisionWebPlugin {
+bool IsRealPlaying
+bool IsOpenSound
+bool IsStartRecord
+bool IsLogin
+Task Logout()
+Task StopRealPlay()
+Task<bool> OpenSound()
+Task<bool> CloseSound()
+Task<bool> CapturePictureAndDownload(string fileName, CancellationToken token)
+Task<bool> CapturePicture(string fileName, CancellationToken token)
+Task<bool> StartRecord()
+Task<bool> StopRecord()
}
class IJSRuntime {
+ValueTask<T> InvokeAsync<T>(string identifier, object[] args)
}
class CancellationToken
HikVisionWebPlugin --> IJSRuntime : uses
HikVisionWebPlugin --> CancellationToken : passes
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In
OpenSound/CloseSound,IsOpenSoundis updated unconditionally; consider setting it based on theretvalue so the state reflects whether the operation actually succeeded. - On
LogoutandStopRealPlay, you resetIsStartRecordbut notIsOpenSound; it may be safer to resetIsOpenSoundas well to avoid stale sound state after the session ends. - The default filename in
capturePicturelacks a.jpgextension whilecapturePictureAndDownloadincludes one; aligning these defaults could make the behavior more consistent for consumers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `OpenSound`/`CloseSound`, `IsOpenSound` is updated unconditionally; consider setting it based on the `ret` value so the state reflects whether the operation actually succeeded.
- On `Logout` and `StopRealPlay`, you reset `IsStartRecord` but not `IsOpenSound`; it may be safer to reset `IsOpenSound` as well to avoid stale sound state after the session ends.
- The default filename in `capturePicture` lacks a `.jpg` extension while `capturePictureAndDownload` includes one; aligning these defaults could make the behavior more consistent for consumers.
## Individual Comments
### Comment 1
<location> `src/components/BootstrapBlazor.HikVision/Components/HikVisionWebPlugin.razor.cs:294-296` </location>
<code_context>
{
var code = await InvokeAsync<int>("openSound", Id);
ret = code == 100;
+ IsOpenSound = true;
}
return ret;
</code_context>
<issue_to_address>
**issue (bug_risk):** Local sound state is set to true even when the JS call fails.
`ret` correctly reflects whether `openSound` succeeded (`code == 100`), but `IsOpenSound` is always set to `true`, so it can become out of sync with the actual sound state when `code != 100`. Set `IsOpenSound = ret` (or only set it when `code == 100`) so it stays consistent with the real state.
</issue_to_address>
### Comment 2
<location> `src/components/BootstrapBlazor.HikVision/Components/HikVisionWebPlugin.razor.cs:310-312` </location>
<code_context>
{
var code = await InvokeAsync<int>("closeSound", Id);
ret = code == 100;
+ IsOpenSound = false;
}
return ret;
</code_context>
<issue_to_address>
**issue (bug_risk):** Local sound state is set to false even if `closeSound` fails.
Because `IsOpenSound` is set to `false` unconditionally, callers may believe sound is closed even when `closeSound` fails. Align the flag with the actual result from `code == 100` (e.g., set `IsOpenSound = ret;` or only update it when the call succeeds) so state remains accurate.
</issue_to_address>
### Comment 3
<location> `src/components/BootstrapBlazor.HikVision/Components/HikVisionWebPlugin.razor.cs:177` </location>
<code_context>
{
await InvokeVoidAsync("logout", Id);
}
+ IsStartRecord = false;
IsRealPlaying = false;
IsLogin = false;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider resetting `IsOpenSound` during logout to avoid stale state.
On logout you reset `IsStartRecord` and `IsRealPlaying`, but leave `IsOpenSound` as-is, so it can remain `true` after the session is torn down. Please also set `IsOpenSound = false` here (and possibly in `StopRealPlay`) so the public state is consistent with a logged-out session.
Suggested implementation:
```csharp
{
await InvokeVoidAsync("logout", Id);
}
IsStartRecord = false;
IsRealPlaying = false;
IsOpenSound = false;
IsLogin = false;
await TriggerLogout();
```
```csharp
{
await InvokeVoidAsync("stopRealPlay", Id);
IsRealPlaying = false;
IsStartRecord = false;
IsOpenSound = false;
await TriggerStopRealPlay();
}
```
</issue_to_address>
### Comment 4
<location> `src/components/BootstrapBlazor.HikVision/wwwroot/hikvision.js:412-421` </location>
<code_context>
}
-export async function capturePicture(id) {
+export function capturePicture(id, szFileName) {
const vision = Data.get(id);
const { realPlaying } = vision;
if (realPlaying !== true) {
- return "";
+ return false;
}
try {
- const base64 = await WebVideoCtrl.I_CapturePicData();
- if (base64) {
- const bytes = base64ToArray(base64);
- return DotNet.createJSStreamReference(bytes.buffer);
+ if (!szFileName) {
+ szFileName = `capture_${new Date().getTime()}`;
}
+ WebVideoCtrl.I_CapturePic(szFileName);
+ return true;
}
</code_context>
<issue_to_address>
**suggestion:** The capture filename here is extension-less, unlike the `.jpg` used in the download variant.
This uses a default of `capture_*` without an extension, whereas `capturePictureAndDownload` uses `capture_*.jpg`. To avoid confusion or downstream issues if consumers expect a `.jpg`, align the default naming here (e.g., also use `.jpg`) or otherwise keep the conventions consistent between the capture methods.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
294
to
+296
| var code = await InvokeAsync<int>("openSound", Id); | ||
| ret = code == 100; | ||
| IsOpenSound = true; |
There was a problem hiding this comment.
issue (bug_risk): Local sound state is set to true even when the JS call fails.
ret correctly reflects whether openSound succeeded (code == 100), but IsOpenSound is always set to true, so it can become out of sync with the actual sound state when code != 100. Set IsOpenSound = ret (or only set it when code == 100) so it stays consistent with the real state.
Comment on lines
310
to
+312
| var code = await InvokeAsync<int>("closeSound", Id); | ||
| ret = code == 100; | ||
| IsOpenSound = false; |
There was a problem hiding this comment.
issue (bug_risk): Local sound state is set to false even if closeSound fails.
Because IsOpenSound is set to false unconditionally, callers may believe sound is closed even when closeSound fails. Align the flag with the actual result from code == 100 (e.g., set IsOpenSound = ret; or only update it when the call succeeds) so state remains accurate.
Comment on lines
+412
to
+421
| export function capturePicture(id, szFileName) { | ||
| const vision = Data.get(id); | ||
| const { realPlaying } = vision; | ||
|
|
||
| if (realPlaying !== true) { | ||
| return ""; | ||
| return false; | ||
| } | ||
|
|
||
| try { | ||
| const base64 = await WebVideoCtrl.I_CapturePicData(); | ||
| if (base64) { | ||
| const bytes = base64ToArray(base64); | ||
| return DotNet.createJSStreamReference(bytes.buffer); | ||
| if (!szFileName) { |
There was a problem hiding this comment.
suggestion: The capture filename here is extension-less, unlike the .jpg used in the download variant.
This uses a default of capture_* without an extension, whereas capturePictureAndDownload uses capture_*.jpg. To avoid confusion or downstream issues if consumers expect a .jpg, align the default naming here (e.g., also use .jpg) or otherwise keep the conventions consistent between the capture methods.
There was a problem hiding this comment.
Pull request overview
This PR refactors the HikVision capture picture functionality by changing from a stream-based approach to a file-based approach. The CapturePicture method now saves images directly to disk using the HikVision SDK's I_CapturePic API instead of returning a stream reference. Additionally, state tracking properties (IsOpenSound, IsStartRecord) are introduced to monitor component state.
- Simplifies the capture picture API by removing stream handling and using direct file saving
- Adds state tracking properties for sound and recording status
- Updates both methods to accept an optional
fileNameparameter and return boolean success indicators
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/components/BootstrapBlazor.HikVision/wwwroot/hikvision.js | Refactored capturePicture to use I_CapturePic (file-based) instead of I_CapturePicData (stream-based); updated capturePictureAndDownload to accept filename parameter and return boolean |
| src/components/BootstrapBlazor.HikVision/Components/HikVisionWebPlugin.razor.cs | Changed CapturePicture return type from IJSStreamReference? to bool; added IsOpenSound and IsStartRecord state properties; removed TriggerReceivePictureStream callback method |
| src/components/BootstrapBlazor.HikVision/BootstrapBlazor.HikVision.csproj | Bumped version from 10.0.7 to 10.0.9 |
Comments suppressed due to low confidence (5)
src/components/BootstrapBlazor.HikVision/wwwroot/hikvision.js:1
- Unused import registerBootstrapBlazorModule.
import { addScript, registerBootstrapBlazorModule } from '../BootstrapBlazor/modules/utility.js';
src/components/BootstrapBlazor.HikVision/wwwroot/hikvision.js:76
- Unused variable originalDestroy.
const originalDestroy = JSVideoPlugin.prototype.JS_DestroyPlugin;
src/components/BootstrapBlazor.HikVision/wwwroot/hikvision.js:190
- Unused variable logined.
const { szDeviceIdentify, logined } = vision;
src/components/BootstrapBlazor.HikVision/wwwroot/hikvision.js:390
- Unused variable iWndIndex.
const { iWndIndex, realPlaying } = vision;
src/components/BootstrapBlazor.HikVision/wwwroot/hikvision.js:433
- Unused variable base64ToArray.
const base64ToArray = base64String => {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const base64 = await WebVideoCtrl.I_CapturePicData(); | ||
| if (base64) { | ||
| if (!szFileName) { | ||
| szFileName = `capture_${new Date().getTime()}.jpg` |
There was a problem hiding this comment.
Missing semicolon at the end of the line. JavaScript requires semicolons to properly terminate statements.
Suggested change
| szFileName = `capture_${new Date().getTime()}.jpg` | |
| szFileName = `capture_${new Date().getTime()}.jpg`; |
| { | ||
| await InvokeVoidAsync("stopRealPlay", Id); | ||
| IsRealPlaying = false; | ||
| IsStartRecord = false; |
There was a problem hiding this comment.
The IsOpenSound state should be reset when stopping real play. When stopping the real-time preview, the sound state should also be reset to false, similar to how IsStartRecord is reset.
Suggested change
| IsStartRecord = false; | |
| IsStartRecord = false; | |
| IsOpenSound = false; |
Comment on lines
348
to
350
| /// 抓图方法返回 <see cref="IJSStreamReference"/> 实例 | ||
| /// </summary> | ||
| /// <returns></returns> |
There was a problem hiding this comment.
The XML documentation comment is outdated. The method no longer returns an IJSStreamReference instance but now returns a boolean value indicating success or failure.
Suggested change
| /// 抓图方法返回 <see cref="IJSStreamReference"/> 实例 | |
| /// </summary> | |
| /// <returns></returns> | |
| /// 抓图方法，返回操作是否成功 | |
| /// </summary> | |
| /// <returns>表示抓图操作是否成功的布尔值。</returns> |
| { | ||
| var code = await InvokeAsync<int>("openSound", Id); | ||
| ret = code == 100; | ||
| IsOpenSound = true; |
There was a problem hiding this comment.
The IsOpenSound property is set to true regardless of whether the operation actually succeeded. It should only be set to true when ret is true (i.e., when code == 100).
Suggested change
| IsOpenSound = true; | |
| IsOpenSound = ret; |
| { | ||
| var code = await InvokeAsync<int>("closeSound", Id); | ||
| ret = code == 100; | ||
| IsOpenSound = false; |
There was a problem hiding this comment.
The IsOpenSound property is set to false regardless of whether the operation actually succeeded. It should only be set to false when ret is true (i.e., when code == 100).
| await InvokeVoidAsync("logout", Id); | ||
| } | ||
| IsStartRecord = false; | ||
| IsRealPlaying = false; |
There was a problem hiding this comment.
The IsOpenSound state is not reset when logout occurs. When a user logs out, the sound state should also be reset to false, similar to how IsStartRecord and IsRealPlaying are reset.
Suggested change
| IsRealPlaying = false; | |
| IsRealPlaying = false; | |
| IsOpenSound = false; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #868
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add support for capturing pictures and downloads in the HikVision web plugin while tracking sound and recording state.
New Features:
Enhancements: