Bug 210335 - Add a timer to AVVideoCaptureSource to verify reception of frames
Summary: Add a timer to AVVideoCaptureSource to verify reception of frames
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-10 08:16 PDT by youenn fablet
Modified: 2020-04-14 02:37 PDT (History)
10 users (show)

See Also:


Attachments
Patch (7.21 KB, patch)
2020-04-10 08:24 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (7.04 KB, patch)
2020-04-10 08:28 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (6.61 KB, patch)
2020-04-11 16:25 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2020-04-10 08:16:03 PDT
Add a timer to AVVideoCaptureSource to verify reception of frames
Comment 1 youenn fablet 2020-04-10 08:24:54 PDT
Created attachment 396085 [details]
Patch
Comment 2 youenn fablet 2020-04-10 08:28:53 PDT
Created attachment 396087 [details]
Patch
Comment 3 youenn fablet 2020-04-11 16:25:58 PDT
Created attachment 396197 [details]
Patch
Comment 4 Eric Carlson 2020-04-13 09:38:20 PDT
Comment on attachment 396197 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=396197&action=review

> Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h:143
> +    static constexpr Seconds verifyCaptureInterval = 3_s;

Is three seconds long enough?

> Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h:144
> +    static const uint64_t framesToDropWhenStarting = 4;

Shouldn't this be relative to the capture rate?

> Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:163
> +    RELEASE_LOG_ERROR(WebRTC, "AVVideoCaptureSource::verifyIsCapturing - no frame received in %d seconds, failing", static_cast<int>(m_verifyCapturingTimer.repeatInterval().value()));
> +    captureFailed();

Failing capture completely is quite severe. Should we try to stop and restart the camera one time?
Comment 5 youenn fablet 2020-04-14 02:32:40 PDT
(In reply to Eric Carlson from comment #4)
> Comment on attachment 396197 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=396197&action=review
> 
> > Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h:143
> > +    static constexpr Seconds verifyCaptureInterval = 3_s;
> 
> Is three seconds long enough?

I tried to pick something not too short but not too long either.
Timer starts when session is running so frames should flow very quickly in theory.


> > Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h:144
> > +    static const uint64_t framesToDropWhenStarting = 4;
> 
> Shouldn't this be relative to the capture rate?

Might be the case.
IIRC, from your testing, the first frames were bad, probably at 30 fps.
I am guessing that we could reduce this number at lower frame rate.

> > Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:163
> > +    RELEASE_LOG_ERROR(WebRTC, "AVVideoCaptureSource::verifyIsCapturing - no frame received in %d seconds, failing", static_cast<int>(m_verifyCapturingTimer.repeatInterval().value()));
> > +    captureFailed();
> 
> Failing capture completely is quite severe. Should we try to stop and
> restart the camera one time?

I was wondering the same thing and went this way following what we are doing for audio capture. At the same time, the application can always call getUserMedia if it wants to restart capture.

Let's try that as a follow-up.
Comment 6 EWS 2020-04-14 02:36:11 PDT
Committed r260064: <https://trac.webkit.org/changeset/260064>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396197 [details].
Comment 7 Radar WebKit Bug Importer 2020-04-14 02:37:13 PDT
<rdar://problem/61762873>