My first guess is that the garbage collection from JS doesn't happen as it
should and the VideoTrackRecorder & co [1] don't get destroyed properly,
holding on to the VideoFrames used for capture.  chfremer@ could you plz
take a look?

[1] https://chromium.googlesource.com/chromium/src/+/HEAD/third_party/blink/renderer/modules/mediarecorder/README.md

That is a very nice repro page :-).

Just checked if I can repro on Linux, but was unable to repro on M69 and M67. Will try on a nautilus tomorrow.

Able to repro on M69 on nautilus. Will investigate for root cause tomorrow.

re #1: Confirmed that the issue is due to no buffers available in the video frame buffer pool, causing all frames to get dropped.

Next will have to look into confirming that it is because VideoTrackRecorder & co don't get destroyed properly, and why it happens.

Ok, here is the root cause analysis. It turns out it is not related to garbage collection not happening, but rather a leak due to self-ownership.

The issue is that it can happen that VideoFrame instances sent to VeaEncoder 
never get released. Here is the problematic sequence:

1. VideoFrame arrives at VideoTrackRecorder::Encoder::StartFrameEncode() [1]
2. The frame is wrapped in another VideoFrame instance and the wrapped frame is
   passed down to VEAEncoder::EncodeOnEncodingTaskRunner(). This wrapping is 
   done in order to register [3] a callback to 
   VideoTrackRecorder::Encoder::FrameReleased to get invoked whenever 
   EncodeOnEncodingTaskRunner() releases the frame. Note, that this callback 
   takes shared ownership of the VEAEncoder instance.
3. VEAEncoder::EncodeOnEncodingTaskRunner() stores the wrapped frame as a member
   |last_frame_| [4]. This is intended to be temporary only until the next 
   invocation of EncodeOnEncodingTaskRunner(). But the problem is that there is 
   no guarantee that another invocation of EncodeOnEncodingTaskRunner() ever
   occurs. This is the case when the recorder is stopped fast enough such that 
   no subsequent frame is processed.
4. The VideoTrackRecorder instance is destroyed, but the VEAEncoder still has 
   and keeps shared ownership of itself. As a result the VEAEncoder instance and
   the VideoFrame it is holding on to in member |last_frame_| are never 
   released.
Once the above has happened 3 times, all video frame buffers are permanently 
blocked and no more frames can be transported.

[1] https://cs.chromium.org/chromium/src/content/renderer/media_recorder/video_track_recorder.cc?g=0&l=189
[2] https://cs.chromium.org/chromium/src/content/renderer/media_recorder/vea_encoder.cc?l=156
[3] https://cs.chromium.org/chromium/src/content/renderer/media_recorder/video_track_recorder.cc?g=0&l=229
[4] https://cs.chromium.org/chromium/src/content/renderer/media_recorder/vea_encoder.cc?l=176

Thanks a lot chfremer@. Theoretically changing scoped_refptr to WeakPtr should work there. Callback ensures that the counter is updated, but when class is to be destroyed, the counter doesn't matter.

I tried it, but unfortunately it turns out to be not that simple. Without that scoped_refptr keeping things alive, it can happen that the VEAEncoder destructor gets called from encoder_task_runner, which causes other threading issues. I was unable to find any quick solution. It looks like there are 3 threads involved. I would have to spend more time building a full understanding of how the threading was intended to work and potentially redesigning that.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/556813c93fa177cdb275674bdd1016f75e674f47

commit 556813c93fa177cdb275674bdd1016f75e674f47
Author: Christian Fremerey <chfremer@chromium.org>
Date: Tue Jul 24 23:57:26 2018

[Media Recorder] Fix video freeze on short recording. Fix tab crash on initialize.

Fixes an issue where VideoTrackRecorder when used in combination with
VEAEncoder and very short recording durations (just 1 frame) would
occasionally hold on to video frames forever, causing frame buffers
from the buffer pool to become permanently blocked, eventually leading
to video capture freezing.

The cause was a circular ownership that would happen if only 1 frame
is recorded.

This CL additionally fixes a crash that could happen on initialization
of VEAEncoder. This issue was revealed during testing of the above fix.
Since it is not 100% if this crash was masked by the other issue, and
would only start occurring with the above fix, I am making a fix for that
part of this CL as well.

The cause for the crash issue was that class VEAEncoder, which uses
RefCountedThreadSafe<>, would post an asynchronous task from its
constructor. If this task would finish executing before the newly
constructed instance was assigned to a scoped_refptr<> by however
called the constructor, the instance would destroy itself, because
of the AddRef() and subsequent Release() done by the posting and
running of the asynchronous task.

The fix for this is to use a new method Initialize() instead of
posting a task from the constructor.

Bug:  859610 
Change-Id: I6645aebfaa7659ab0ced0a20fae57eedfb9d84ab
Reviewed-on: https://chromium-review.googlesource.com/1145877
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577735}
[modify] https://crrev.com/556813c93fa177cdb275674bdd1016f75e674f47/content/renderer/media_recorder/vea_encoder.cc
[modify] https://crrev.com/556813c93fa177cdb275674bdd1016f75e674f47/content/renderer/media_recorder/vea_encoder.h
[modify] https://crrev.com/556813c93fa177cdb275674bdd1016f75e674f47/content/renderer/media_recorder/video_track_recorder.cc
[modify] https://crrev.com/556813c93fa177cdb275674bdd1016f75e674f47/content/renderer/media_recorder/video_track_recorder.h
[modify] https://crrev.com/556813c93fa177cdb275674bdd1016f75e674f47/content/renderer/media_recorder/video_track_recorder_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a9fb21d345e9fcc11105b1d33a3cb5c8b27bcafe

commit a9fb21d345e9fcc11105b1d33a3cb5c8b27bcafe
Author: Yoichi Osato <yoichio@chromium.org>
Date: Thu Jul 26 03:13:31 2018

Revert "[Media Recorder] Fix video freeze on short recording. Fix tab crash on initialize."

This reverts commit 556813c93fa177cdb275674bdd1016f75e674f47.

Reason for revert: LeakSanitizer detected memory leaks
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8940004334055018640/+/steps/content_unittests__with_patch_/0/logs/VideoTrackRecorderTest.HandlesOnError/0

Original change's description:
> [Media Recorder] Fix video freeze on short recording. Fix tab crash on initialize.
> 
> Fixes an issue where VideoTrackRecorder when used in combination with
> VEAEncoder and very short recording durations (just 1 frame) would
> occasionally hold on to video frames forever, causing frame buffers
> from the buffer pool to become permanently blocked, eventually leading
> to video capture freezing.
> 
> The cause was a circular ownership that would happen if only 1 frame
> is recorded.
> 
> This CL additionally fixes a crash that could happen on initialization
> of VEAEncoder. This issue was revealed during testing of the above fix.
> Since it is not 100% if this crash was masked by the other issue, and
> would only start occurring with the above fix, I am making a fix for that
> part of this CL as well.
> 
> The cause for the crash issue was that class VEAEncoder, which uses
> RefCountedThreadSafe<>, would post an asynchronous task from its
> constructor. If this task would finish executing before the newly
> constructed instance was assigned to a scoped_refptr<> by however
> called the constructor, the instance would destroy itself, because
> of the AddRef() and subsequent Release() done by the posting and
> running of the asynchronous task.
> 
> The fix for this is to use a new method Initialize() instead of
> posting a task from the constructor.
> 
> Bug:  859610 
> Change-Id: I6645aebfaa7659ab0ced0a20fae57eedfb9d84ab
> Reviewed-on: https://chromium-review.googlesource.com/1145877
> Commit-Queue: Christian Fremerey <chfremer@chromium.org>
> Reviewed-by: Emircan Uysaler <emircan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#577735}

TBR=emircan@chromium.org,chfremer@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  859610 ,867625
Change-Id: If3fed5676c76da0d6b6ef6a041d74c782e87ebf3
Reviewed-on: https://chromium-review.googlesource.com/1150947
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578181}
[modify] https://crrev.com/a9fb21d345e9fcc11105b1d33a3cb5c8b27bcafe/content/renderer/media_recorder/vea_encoder.cc
[modify] https://crrev.com/a9fb21d345e9fcc11105b1d33a3cb5c8b27bcafe/content/renderer/media_recorder/vea_encoder.h
[modify] https://crrev.com/a9fb21d345e9fcc11105b1d33a3cb5c8b27bcafe/content/renderer/media_recorder/video_track_recorder.cc
[modify] https://crrev.com/a9fb21d345e9fcc11105b1d33a3cb5c8b27bcafe/content/renderer/media_recorder/video_track_recorder.h
[modify] https://crrev.com/a9fb21d345e9fcc11105b1d33a3cb5c8b27bcafe/content/renderer/media_recorder/video_track_recorder_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/545410e67c57571be348780766ef40ead5addba9

commit 545410e67c57571be348780766ef40ead5addba9
Author: Christian Fremerey <chfremer@chromium.org>
Date: Wed Aug 08 20:00:37 2018

Reland [Media Recorder] Fix video freeze on short recording. Fix tab crash on initialize.

Patch Set 1 is the original CL that got reverted.
Patch Set 2 fixes the reason for the revert.

Reason for the revert was a memory leak during a unit test. This was caused by
a newly added DeleteSoon() causing the test to exist before all objects had
been released. The fix is to add a ScopedTaskEnvironment::RunUntilIdle() at
test teardown.

Original CL description:

Fixes an issue where VideoTrackRecorder when used in combination with
VEAEncoder and very short recording durations (just 1 frame) would
occasionally hold on to video frames forever, causing frame buffers
from the buffer pool to become permanently blocked, eventually leading
to video capture freezing.

The cause was a circular ownership that would happen if only 1 frame
is recorded.

This CL additionally fixes a crash that could happen on initialization
of VEAEncoder. This issue was revealed during testing of the above fix.
Since it is not 100% if this crash was masked by the other issue, and
would only start occurring with the above fix, I am making a fix for that
part of this CL as well.

The cause for the crash issue was that class VEAEncoder, which uses
RefCountedThreadSafe<>, would post an asynchronous task from its
constructor. If this task would finish executing before the newly
constructed instance was assigned to a scoped_refptr<> by however
called the constructor, the instance would destroy itself, because
of the AddRef() and subsequent Release() done by the posting and
running of the asynchronous task.

The fix for this is to use a new method Initialize() instead of
posting a task from the constructor.

TBR=emircan@chromium.org

Bug:  859610 
Change-Id: I120b60b4b5cdf7462e38a74b7c757af6b5e1a7d6
Reviewed-on: https://chromium-review.googlesource.com/1167742
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581672}
[modify] https://crrev.com/545410e67c57571be348780766ef40ead5addba9/content/renderer/media_recorder/vea_encoder.cc
[modify] https://crrev.com/545410e67c57571be348780766ef40ead5addba9/content/renderer/media_recorder/vea_encoder.h
[modify] https://crrev.com/545410e67c57571be348780766ef40ead5addba9/content/renderer/media_recorder/video_track_recorder.cc
[modify] https://crrev.com/545410e67c57571be348780766ef40ead5addba9/content/renderer/media_recorder/video_track_recorder.h
[modify] https://crrev.com/545410e67c57571be348780766ef40ead5addba9/content/renderer/media_recorder/video_track_recorder_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/91a3bbb5e55c56385300dc2a112b1847b2de5f16

commit 91a3bbb5e55c56385300dc2a112b1847b2de5f16
Author: Christian Fremerey <chfremer@chromium.org>
Date: Wed Aug 08 20:15:06 2018

Revert "Reland [Media Recorder] Fix video freeze on short recording. Fix tab crash on initialize."

This reverts commit 545410e67c57571be348780766ef40ead5addba9.

Reason for revert: A different fix for the initialization had already landed by now, see https://chromium-review.googlesource.com/c/chromium/src/+/1158316. I need to remove the extra initialization changes from my CL.

Original change's description:
> Reland [Media Recorder] Fix video freeze on short recording. Fix tab crash on initialize.
> 
> Patch Set 1 is the original CL that got reverted.
> Patch Set 2 fixes the reason for the revert.
> 
> Reason for the revert was a memory leak during a unit test. This was caused by
> a newly added DeleteSoon() causing the test to exist before all objects had
> been released. The fix is to add a ScopedTaskEnvironment::RunUntilIdle() at
> test teardown.
> 
> Original CL description:
> 
> Fixes an issue where VideoTrackRecorder when used in combination with
> VEAEncoder and very short recording durations (just 1 frame) would
> occasionally hold on to video frames forever, causing frame buffers
> from the buffer pool to become permanently blocked, eventually leading
> to video capture freezing.
> 
> The cause was a circular ownership that would happen if only 1 frame
> is recorded.
> 
> This CL additionally fixes a crash that could happen on initialization
> of VEAEncoder. This issue was revealed during testing of the above fix.
> Since it is not 100% if this crash was masked by the other issue, and
> would only start occurring with the above fix, I am making a fix for that
> part of this CL as well.
> 
> The cause for the crash issue was that class VEAEncoder, which uses
> RefCountedThreadSafe<>, would post an asynchronous task from its
> constructor. If this task would finish executing before the newly
> constructed instance was assigned to a scoped_refptr<> by however
> called the constructor, the instance would destroy itself, because
> of the AddRef() and subsequent Release() done by the posting and
> running of the asynchronous task.
> 
> The fix for this is to use a new method Initialize() instead of
> posting a task from the constructor.
> 
> TBR=emircan@chromium.org
> 
> Bug:  859610 
> Change-Id: I120b60b4b5cdf7462e38a74b7c757af6b5e1a7d6
> Reviewed-on: https://chromium-review.googlesource.com/1167742
> Reviewed-by: Christian Fremerey <chfremer@chromium.org>
> Commit-Queue: Christian Fremerey <chfremer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#581672}

TBR=emircan@chromium.org,chfremer@chromium.org

Change-Id: I4794d8d76b78bd1ab86860230bc01f0f20d3bd18
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  859610 
Reviewed-on: https://chromium-review.googlesource.com/1167768
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581679}
[modify] https://crrev.com/91a3bbb5e55c56385300dc2a112b1847b2de5f16/content/renderer/media_recorder/vea_encoder.cc
[modify] https://crrev.com/91a3bbb5e55c56385300dc2a112b1847b2de5f16/content/renderer/media_recorder/vea_encoder.h
[modify] https://crrev.com/91a3bbb5e55c56385300dc2a112b1847b2de5f16/content/renderer/media_recorder/video_track_recorder.cc
[modify] https://crrev.com/91a3bbb5e55c56385300dc2a112b1847b2de5f16/content/renderer/media_recorder/video_track_recorder.h
[modify] https://crrev.com/91a3bbb5e55c56385300dc2a112b1847b2de5f16/content/renderer/media_recorder/video_track_recorder_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0013a455ba849543cf6f3024f8ede9b27827e947

commit 0013a455ba849543cf6f3024f8ede9b27827e947
Author: Christian Fremerey <chfremer@chromium.org>
Date: Mon Aug 13 23:55:17 2018

Reland #2 [Media Recorder] Fix video freeze on short recording.

Reason for the revert was duplicate initialization that happened because a
different fix for the initialization issue had already landed in the master
branch.

Original CL description:

Fixes an issue where VideoTrackRecorder when used in combination with
VEAEncoder and very short recording durations (just 1 frame) would
occasionally hold on to video frames forever, causing frame buffers
from the buffer pool to become permanently blocked, eventually leading
to video capture freezing.

The cause was a circular ownership that would happen if only 1 frame
is recorded.

This CL additionally fixes a crash that could happen on initialization
of VEAEncoder. This issue was revealed during testing of the above fix.
Since it is not 100% if this crash was masked by the other issue, and
would only start occurring with the above fix, I am making a fix for that
part of this CL as well.

The cause for the crash issue was that class VEAEncoder, which uses
RefCountedThreadSafe<>, would post an asynchronous task from its
constructor. If this task would finish executing before the newly
constructed instance was assigned to a scoped_refptr<> by however
called the constructor, the instance would destroy itself, because
of the AddRef() and subsequent Release() done by the posting and
running of the asynchronous task.

The fix for this is to use a new method Initialize() instead of
posting a task from the constructor.

TBR=emircan@chromium.org

Bug:  859610 
Change-Id: I77d6865be574bf86afca7d0908ce2339f6ecd91e
Reviewed-on: https://chromium-review.googlesource.com/1173408
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582753}
[modify] https://crrev.com/0013a455ba849543cf6f3024f8ede9b27827e947/content/renderer/media_recorder/vea_encoder.cc
[modify] https://crrev.com/0013a455ba849543cf6f3024f8ede9b27827e947/content/renderer/media_recorder/vea_encoder.h
[modify] https://crrev.com/0013a455ba849543cf6f3024f8ede9b27827e947/content/renderer/media_recorder/video_track_recorder.cc
[modify] https://crrev.com/0013a455ba849543cf6f3024f8ede9b27827e947/content/renderer/media_recorder/video_track_recorder.h
[modify] https://crrev.com/0013a455ba849543cf6f3024f8ede9b27827e947/content/renderer/media_recorder/video_track_recorder_unittest.cc