feat(HikVision): add CapturePicture function #865
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 HikVision capture-picture functionality (both download and stream-to-.NET variants) and aligns JS and C# sound/volume APIs to use a success code of 100 instead of 0. Sequence diagram for HikVision CapturePicture streaming to .NETsequenceDiagram
participant User
participant BlazorComponent as HikVisionWebPlugin
participant JSRuntime
participant JsBridge as HikVisionWebPlugin_razor_js
participant CoreJs as hikvision_js
participant WebVideoCtrl
User->>BlazorComponent: CapturePicture(token)
alt IsLogin && IsRealPlaying
BlazorComponent->>BlazorComponent: create TaskCompletionSource
BlazorComponent->>JSRuntime: InvokeVoidAsync(capturePicture, token, Id)
JSRuntime->>JsBridge: capturePicture(Id)
JsBridge->>CoreJs: capturePicture(Id)
CoreJs->>CoreJs: validate realPlaying
alt realPlaying is true
CoreJs->>WebVideoCtrl: I_CapturePicData()
WebVideoCtrl-->>CoreJs: base64Image
CoreJs->>CoreJs: base64ToArray(base64Image)
CoreJs-->>JSRuntime: JSStreamReference(bytes.buffer)
JSRuntime-->>BlazorComponent: TriggerReceivePictureStream(stream)
BlazorComponent->>BlazorComponent: _captureTaskCompletionSource.SetResult(stream)
BlazorComponent-->>User: IJSStreamReference
else realPlaying is not true
CoreJs-->>JSRuntime: emptyString
JSRuntime-->>BlazorComponent: TriggerReceivePictureStream(null)
BlazorComponent-->>User: null
end
else Not logged in or not playing
BlazorComponent-->>User: null
end
Updated class diagram for HikVisionWebPlugin capture and sound APIsclassDiagram
class HikVisionWebPlugin {
+string Id
+bool IsLogin
+bool IsRealPlaying
-TaskCompletionSource~IJSStreamReference?~ _captureTaskCompletionSource
+Task~bool~ OpenSound()
+Task~bool~ CloseSound()
+Task~bool~ SetVolume(int value)
+Task CapturePictureAndDownload()
+Task~IJSStreamReference?~ CapturePicture(CancellationToken token)
+Task TriggerReceivePictureStream(IJSStreamReference stream)
}
class IJSStreamReference {
<<interface>>
}
class TaskCompletionSource~T~ {
+void SetResult(T result)
+void SetException(Exception exception)
+Task~T~ Task
}
class CancellationToken {
}
HikVisionWebPlugin --> IJSStreamReference : uses
HikVisionWebPlugin --> TaskCompletionSource~IJSStreamReference?~ : manages
HikVisionWebPlugin --> CancellationToken : parameter
Flow diagram for capturePictureAndDownload in HikVision JSflowchart TD
A["Start capturePictureAndDownload(id)"] --> B["vision = Data.get(id)"]
B --> C{"vision.realPlaying === true?"}
C -- No --> Z["Return"]
C -- Yes --> D["Call WebVideoCtrl.I_CapturePicData()"]
D --> E{"base64 result exists?"}
E -- No --> Z
E -- Yes --> F["Create anchor element"]
F --> G["Set href to data:image/jpg;base64,base64"]
G --> H["Set download filename to capture_timestamp.jpg"]
H --> I["Append anchor to document.body"]
I --> J["Trigger anchor.click()"]
J --> K["Remove anchor from document.body"]
K --> Z
D -->|Exception| Z
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:
- The new CapturePicture interop path looks inconsistent: the JS
capturePicturefunction just returns aJSStreamReferencebut is invoked viaInvokeVoidAsyncin C#, and the[JSInvokable] TriggerReceivePictureStreamcallback is never called from JS, so the TaskCompletionSource will never complete as currently wired. - The
capturePictureJS function is declared beforebase64ToArrayis defined as aconstarrow function; becauseconstdeclarations are not hoisted, callingbase64ToArraythere will throw at runtime—either move the helper above or change it to a hoisted function declaration. - In
CapturePicture, the cancellation token is passed to JS (InvokeVoidAsync("capturePicture", token, Id)), but the JS function signature expects onlyid, so the arguments are shifted andidreceives the token rather than the HikVision instance id.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new CapturePicture interop path looks inconsistent: the JS `capturePicture` function just returns a `JSStreamReference` but is invoked via `InvokeVoidAsync` in C#, and the `[JSInvokable] TriggerReceivePictureStream` callback is never called from JS, so the TaskCompletionSource will never complete as currently wired.
- The `capturePicture` JS function is declared before `base64ToArray` is defined as a `const` arrow function; because `const` declarations are not hoisted, calling `base64ToArray` there will throw at runtime—either move the helper above or change it to a hoisted function declaration.
- In `CapturePicture`, the cancellation token is passed to JS (`InvokeVoidAsync("capturePicture", token, Id)`), but the JS function signature expects only `id`, so the arguments are shifted and `id` receives the token rather than the HikVision instance id.
## Individual Comments
### Comment 1
<location> `src/components/BootstrapBlazor.HikVision/Components/HikVisionWebPlugin.razor.cs:331-340` </location>
<code_context>
+ private TaskCompletionSource<IJSStreamReference?>? _captureTaskCompletionSource = null;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The single `_captureTaskCompletionSource` field can cause races or leaks if multiple captures are started or never completed.
Because `_captureTaskCompletionSource` is shared and mutable, concurrent `CapturePicture` calls will overwrite each other’s TCS and the last JS callback wins. If JS never calls back (e.g., error/navigation), the TCS can also remain uncompleted and hold state. If concurrent invocations are possible, either (1) prevent parallel captures (e.g., fail fast when one is in progress), or (2) scope a TCS per call (e.g., local TCS keyed to the JS callback). Also ensure cancellation reliably completes the TCS so callers don’t hang.
Suggested implementation:
```csharp
/// <summary>
/// 抓图方法返回 Url
/// </summary>
/// <returns></returns>
public async Task<IJSStreamReference?> CapturePicture(CancellationToken token = default)
{
if (!IsLogin || !IsRealPlaying)
{
return null;
}
// Fail fast if a previous capture is still in progress to avoid races on _captureTaskCompletionSource
if (_captureTaskCompletionSource is { Task.IsCompleted: false })
{
throw new InvalidOperationException("A capture operation is already in progress.");
}
var tcs = new TaskCompletionSource<IJSStreamReference?>(TaskCreationOptions.RunContinuationsAsynchronously);
_captureTaskCompletionSource = tcs;
using var registration = token.Register(static state =>
{
var source = (TaskCompletionSource<IJSStreamReference?>)state!;
source.TrySetCanceled();
}, tcs);
// This JS call should eventually invoke a .NET callback that completes _captureTaskCompletionSource
await InvokeVoidAsync("capturePicture", Id);
return await tcs.Task.ConfigureAwait(false);
```
To fully implement the behavior described in your review comment, you will also need to:
1. **Complete and clear the TCS from the JS callback**
Wherever the JS interop callback for a successful capture is currently handled (e.g., a method invoked from JS with `[JSInvokable]`), ensure it:
- Calls `_captureTaskCompletionSource?.TrySetResult(streamReference);`
- Sets `_captureTaskCompletionSource = null;` once it has completed (in success, error, or cancellation paths).
2. **Handle JS-side errors or timeouts**
If you have a JS-side failure path (e.g., error in the HikVision plugin, user denies permission, or navigation occurs), call back into .NET and:
- Either `TrySetException(...)` (with a meaningful exception) or `TrySetResult(null)` depending on how you want to signal failure.
- Always clear the field: `_captureTaskCompletionSource = null;`.
3. **Align the JS function name if needed**
The snippet above calls `InvokeVoidAsync("capturePicture", Id);`.
- If your actual JS function has a different name (e.g., currently you might be using `capturePictureAndDownload` for the URL-returning variant), replace `"capturePicture"` with the correct JS function name in the `InvokeVoidAsync` call.
</issue_to_address>
### Comment 2
<location> `src/components/BootstrapBlazor.HikVision/wwwroot/hikvision.js:412-430` </location>
<code_context>
return code;
}
+export async function capturePicture(id) {
+ const vision = Data.get(id);
+ const { realPlaying } = vision;
+
+ if (realPlaying !== true) {
+ return "";
+ }
+
+ try {
+ const base64 = await WebVideoCtrl.I_CapturePicData();
+ if (base64) {
+ const bytes = base64ToArray(base64);
+ return DotNet.createJSStreamReference(bytes.buffer);
+ }
+ }
+ catch (ex) {
+ return null;
+ }
</code_context>
<issue_to_address>
**suggestion:** `capturePicture` returns different types (`""`, `null`, `undefined`, or a stream) depending on the path, which complicates consumers.
The function can currently return:
- `""` when `realPlaying !== true`
- `null` when an exception is thrown
- a `JSStreamReference` when `base64` is truthy
- `undefined` when `realPlaying === true` but `base64` is falsy and no exception occurs
This makes the result harder to handle on the .NET side. Consider normalizing the API so it always returns either a `JSStreamReference` on success or a single, consistent sentinel (`null` or `undefined`) on all non-success paths. If the intent is to always use a callback into .NET, you could also make this function `void` and send all data via the callback instead of mixing return values and callbacks.
```suggestion
export async function capturePicture(id) {
const vision = Data.get(id);
const { realPlaying } = vision;
// Only attempt capture when we are actually playing
if (realPlaying !== true) {
return null;
}
try {
const base64 = await WebVideoCtrl.I_CapturePicData();
if (base64) {
const bytes = base64ToArray(base64);
return DotNet.createJSStreamReference(bytes.buffer);
}
// Normalized non-success result when capture returns no data
return null;
} catch (ex) {
// Normalized non-success result when an error occurs
return null;
}
}
```
</issue_to_address>
### Comment 3
<location> `src/components/BootstrapBlazor.HikVision/wwwroot/hikvision.js:426-429` </location>
<code_context>
+ return DotNet.createJSStreamReference(bytes.buffer);
+ }
+ }
+ catch (ex) {
+ return null;
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Swallowing errors in `capturePictureAndDownload` hides failures and makes diagnosing issues difficult.
The empty `catch` means any exceptions from `I_CapturePicData`, DOM manipulation, or `click()` are silently ignored and the method just returns `null`. Please either log the exception (e.g. `console.error('capturePictureAndDownload failed', ex);`) or propagate a clear failure signal back to .NET, consistent with your other APIs, so callers can react appropriately.
```suggestion
}
catch (ex) {
console.error('capturePictureAndDownload failed', ex);
return null;
}
```
</issue_to_address>
### Comment 4
<location> `src/components/BootstrapBlazor.HikVision/Components/HikVisionWebPlugin.razor.cs:337` </location>
<code_context>
+ /// 抓图方法返回 Url
+ /// </summary>
+ /// <returns></returns>
+ public async Task<IJSStreamReference?> CapturePicture(CancellationToken token = default)
+ {
+ IJSStreamReference? ret = null;
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new capture APIs by returning the JS stream directly from `CapturePicture` instead of using a shared `TaskCompletionSource` and JS-invokable callback, and by fixing or removing the unnecessary `CancellationToken` parameter.
The new capture APIs add avoidable state and complexity. You can keep the functionality while simplifying the flow and removing concurrency hazards.
### 1. Remove the shared `TaskCompletionSource` and callback
Instead of a field-level `TaskCompletionSource` plus `[JSInvokable]` callback, you can directly await the JS result. This avoids:
- global mutable state (`_captureTaskCompletionSource`)
- race conditions if `CapturePicture` is called concurrently
- extra JS → .NET callback plumbing
Assuming your JS `capturePicture` function can return a stream reference (as the other reviewer mentioned), you can simplify to:
```csharp
// Remove this field entirely
// private TaskCompletionSource<IJSStreamReference?>? _captureTaskCompletionSource = null;
// Remove this method entirely
// [JSInvokable]
// public Task TriggerReceivePictureStream(IJSStreamReference stream) { ... }
public async Task<IJSStreamReference?> CapturePicture(CancellationToken token = default)
{
if (!IsLogin || !IsRealPlaying)
{
return null;
}
// Directly return the JS stream reference
return await InvokeAsync<IJSStreamReference?>("capturePicture", token, Id);
}
```
On the JS side, ensure `capturePicture` returns the stream directly, e.g.:
```js
// Example shape
export function capturePicture(id) {
const bytes = /* capture bytes for id */;
return DotNet.createJSStreamReference(bytes.buffer);
}
```
### 2. Use the `CancellationToken` correctly (or drop it)
Currently:
```csharp
await InvokeVoidAsync("capturePicture", token, Id);
```
passes the token as a JS argument, not as the cancellation parameter. If you keep cancellation support, wire it through the interop correctly:
```csharp
public Task<IJSStreamReference?> CapturePicture(CancellationToken token = default)
{
if (!IsLogin || !IsRealPlaying)
{
return Task.FromResult<IJSStreamReference?>(null);
}
// token is passed as the cancellation token parameter, not as JS argument
return InvokeAsync<IJSStreamReference?>("capturePicture", token, Id);
}
```
If you don’t actually need cancellation here, consider removing the `CancellationToken` from the signature entirely to reduce cognitive overhead:
```csharp
public Task<IJSStreamReference?> CapturePicture()
{
if (!IsLogin || !IsRealPlaying)
{
return Task.FromResult<IJSStreamReference?>(null);
}
return InvokeAsync<IJSStreamReference?>("capturePicture", Id);
}
```
These changes preserve the feature (returning an `IJSStreamReference` from JS) while significantly reducing statefulness and complexity.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| /// 抓图方法返回 Url | ||
| /// </summary> | ||
| /// <returns></returns> | ||
| public async Task<IJSStreamReference?> CapturePicture(CancellationToken token = default) |
There was a problem hiding this comment.
issue (complexity): Consider simplifying the new capture APIs by returning the JS stream directly from CapturePicture instead of using a shared TaskCompletionSource and JS-invokable callback, and by fixing or removing the unnecessary CancellationToken parameter.
The new capture APIs add avoidable state and complexity. You can keep the functionality while simplifying the flow and removing concurrency hazards.
1. Remove the shared TaskCompletionSource and callback
Instead of a field-level TaskCompletionSource plus [JSInvokable] callback, you can directly await the JS result. This avoids:
- global mutable state (
_captureTaskCompletionSource) - race conditions if
CapturePictureis called concurrently - extra JS → .NET callback plumbing
Assuming your JS capturePicture function can return a stream reference (as the other reviewer mentioned), you can simplify to:
// Remove this field entirely
// private TaskCompletionSource<IJSStreamReference?>? _captureTaskCompletionSource = null;
// Remove this method entirely
// [JSInvokable]
// public Task TriggerReceivePictureStream(IJSStreamReference stream) { ... }
public async Task<IJSStreamReference?> CapturePicture(CancellationToken token = default)
{
if (!IsLogin || !IsRealPlaying)
{
return null;
}
// Directly return the JS stream reference
return await InvokeAsync<IJSStreamReference?>("capturePicture", token, Id);
}On the JS side, ensure capturePicture returns the stream directly, e.g.:
// Example shape
export function capturePicture(id) {
const bytes = /* capture bytes for id */;
return DotNet.createJSStreamReference(bytes.buffer);
}2. Use the CancellationToken correctly (or drop it)
Currently:
await InvokeVoidAsync("capturePicture", token, Id);passes the token as a JS argument, not as the cancellation parameter. If you keep cancellation support, wire it through the interop correctly:
public Task<IJSStreamReference?> CapturePicture(CancellationToken token = default)
{
if (!IsLogin || !IsRealPlaying)
{
return Task.FromResult<IJSStreamReference?>(null);
}
// token is passed as the cancellation token parameter, not as JS argument
return InvokeAsync<IJSStreamReference?>("capturePicture", token, Id);
}If you don’t actually need cancellation here, consider removing the CancellationToken from the signature entirely to reduce cognitive overhead:
public Task<IJSStreamReference?> CapturePicture()
{
if (!IsLogin || !IsRealPlaying)
{
return Task.FromResult<IJSStreamReference?>(null);
}
return InvokeAsync<IJSStreamReference?>("capturePicture", Id);
}These changes preserve the feature (returning an IJSStreamReference from JS) while significantly reducing statefulness and complexity.
There was a problem hiding this comment.
Pull request overview
This PR adds picture capture functionality to the HikVision component, allowing users to capture still images from video streams. The implementation includes two capture methods: one that returns a stream reference for programmatic use, and another that directly triggers a browser download.
Key Changes
- Added
CapturePicture()method that returns anIJSStreamReferencefor capturing images programmatically - Added
CapturePictureAndDownload()method that captures and immediately downloads the image - Changed success return codes from
0to100for consistency with error codes (101for precondition failure,102for execution error)
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
wwwroot/hikvision.js |
Implements capturePicture() and capturePictureAndDownload() functions, adds base64ToArray() helper, and updates return codes from 0 to 100 for openSound, closeSound, and setVolume |
Components/HikVisionWebPlugin.razor.js |
Exports the new capture functions to make them available to the Blazor component |
Components/HikVisionWebPlugin.razor.cs |
Adds C# wrapper methods for the capture functionality and updates return code validation from == 0 to == 100 |
BootstrapBlazor.HikVision.csproj |
Bumps version from 10.0.5 to 10.0.6 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+344
to
+352
| try | ||
| { | ||
| await InvokeVoidAsync("capturePicture", token, Id); | ||
| ret = await _captureTaskCompletionSource.Task; | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _captureTaskCompletionSource.SetException(ex); | ||
| } |
There was a problem hiding this comment.
The exception handling logic is incorrect. When an exception occurs, the code sets the exception on the TaskCompletionSource (line 351), but then continues execution and returns ret (which is null). However, since the TaskCompletionSource already has an exception set, awaiting it on line 347 will throw. This creates inconsistent behavior. Either remove the try-catch and let exceptions propagate naturally, or handle them properly without setting the exception on TaskCompletionSource. Note: This issue will be resolved when fixing the async pattern as mentioned in the other comment.
Suggested change
| try | |
| { | |
| await InvokeVoidAsync("capturePicture", token, Id); | |
| ret = await _captureTaskCompletionSource.Task; | |
| } | |
| catch (Exception ex) | |
| { | |
| _captureTaskCompletionSource.SetException(ex); | |
| } | |
| await InvokeVoidAsync("capturePicture", token, Id); | |
| ret = await _captureTaskCompletionSource.Task; |
| const { realPlaying } = vision; | ||
|
|
||
| if (realPlaying !== true) { | ||
| return ""; |
There was a problem hiding this comment.
When realPlaying is not true, the function returns an empty string, but when other conditions fail it returns null. For consistency and to match the C# signature which expects IJSStreamReference?, this should also return null instead of an empty string when not playing.
Suggested change
| return ""; | |
| return null; |
| if (base64) { | ||
| const bytes = base64ToArray(base64); | ||
| return DotNet.createJSStreamReference(bytes.buffer); | ||
| } |
There was a problem hiding this comment.
The function doesn't explicitly return a value when base64 is falsy (line 422-425). This results in an implicit undefined return. For clarity and consistency with error handling (line 428), explicitly return null after the if block to handle the case where I_CapturePicData() returns a falsy value.
Suggested change
| } | |
| } | |
| return null; |
| const base64 = await WebVideoCtrl.I_CapturePicData(); | ||
| if (base64) { | ||
| const anchorElement = document.createElement('a'); | ||
| anchorElement.href = `data:image/jpg;base64,${base64}`; |
There was a problem hiding this comment.
The MIME type specified in the data URI is hardcoded as 'image/jpg', but the correct MIME type for JPEG images is 'image/jpeg'. While some browsers may accept 'image/jpg', using the standard 'image/jpeg' ensures better compatibility.
Suggested change
| anchorElement.href = `data:image/jpg;base64,${base64}`; | |
| anchorElement.href = `data:image/jpeg;base64,${base64}`; |
Comment on lines
+346
to
+347
| await InvokeVoidAsync("capturePicture", token, Id); | ||
| ret = await _captureTaskCompletionSource.Task; |
There was a problem hiding this comment.
The async pattern implemented here is incorrect. The JavaScript function capturePicture returns a value directly (either a JSStreamReference, empty string, or null), so this should use InvokeAsync<IJSStreamReference?> instead of InvokeVoidAsync followed by awaiting a TaskCompletionSource. The current implementation will never resolve because JavaScript never calls TriggerReceivePictureStream. Change line 346 to: ret = await InvokeAsync<IJSStreamReference?>("capturePicture", token, Id); and remove the need for TaskCompletionSource and TriggerReceivePictureStream method.
Comment on lines
+357
to
+369
| /// <summary> | ||
| /// 抓图返回文件流方法 由 Javascript 调用 | ||
| /// </summary> | ||
| /// <param name="stream"></param> | ||
| /// <returns></returns> | ||
| [JSInvokable] | ||
| public async Task TriggerReceivePictureStream(IJSStreamReference stream) | ||
| { | ||
| if (_captureTaskCompletionSource != null) | ||
| { | ||
| _captureTaskCompletionSource.SetResult(stream); | ||
| } | ||
| } |
There was a problem hiding this comment.
The TriggerReceivePictureStream method is never called from JavaScript, making it dead code. Since the capturePicture JavaScript function returns a value directly rather than using a callback pattern, this method should be removed along with the _captureTaskCompletionSource field (line 331).
Comment on lines
+333
to
+335
| /// <summary> | ||
| /// 抓图方法返回 Url | ||
| /// </summary> |
There was a problem hiding this comment.
The comment incorrectly states "抓图方法返回 Url" (Capture method returns URL), but the method actually returns an IJSStreamReference, not a URL. The comment should be updated to reflect that it returns a stream reference.
Comment on lines
+319
to
+322
| /// <summary> | ||
| /// 抓图方法返回 Url | ||
| /// </summary> | ||
| /// <returns></returns> |
There was a problem hiding this comment.
The comment incorrectly states "抓图方法返回 Url" (Capture method returns URL), but this method triggers a download and returns void, not a URL. The comment should be updated to accurately describe that it captures and downloads the picture.
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 #864
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add HikVision capture picture support and adjust sound operation result handling.
New Features:
Enhancements: