Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.

Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/168f828cac74cd3e60cf53cb9c6b7b1e3561c2f4 (Cleanup a bunch of stale expectations and style around empty packets.).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.

+wolenetz. Looks like ChunkDemuxer is vending 0 sized packets? Is that expected?

Hmm crbug.com/663438 documents similar question. I'd need to look further to see if it's still necessary, but perhaps 0-byte packet data was allowed so long as non-empty side-data was there, to support ALPHA channel on VP8 decode using vpx_video_decoder (https://codereview.chromium.org/12263013). I'm not at all sure if that case is one that's required still, and if it is, can it be scoped to a particular decoder.

+vigneshv, do we ever have vp8a/vp9a packets that are side-data-only?

> do we ever have vp8a/vp9a packets that are side-data-only?

no. side data just carries the alpha plane and it must always be accompanied with the Y, U and V planes (the main packet data) for us to decode and render the frame as a whole.

Thanks vigneshv@ -- Dale, I can take this over and fix the MSE demux to skip over 0-byte buffers if you wish. A subsequent cleanup might be good to do later (mojo IPC shouldn't need to handle 0-byte demuxed buffers any more (xhwang@), and possibly something sandersd@ might land soon that he mentioned also may map shared memory of the data -- even 0 byte buffers).

Okay, thanks Matt. Grab this from me if you have time, otherwise I'll take a look next week after promo obligations recede.

Local repro confirms the obvious:
MSE returns a 0-sized data DecoderBuffer from a read.

bufferOnBufferReady<video>: 0, timestamp=179000 duration=3596000 size=0 side_data_size=0 is_key_frame=1 encrypted=0 discard_padding (ms)=(0, 0)

Subsequent attempt to decode with FFVD fails DCHECK_GT:
FATAL:ffmpeg_video_decoder.cc(339)] Check failed: packet.size > 0 (0 vs. 0)

Assuming we want to go the route of continuing to buffer empty buffers in MSE for the purpose of retaining "time-stampiness" of such buffers, adapting the output of SBS::GetNextBuffer[Internal] to be that of the next non-empty buffer, if any, is a bit complex: those methods have complex logic around handling reads involving |track_buffer_| vs |selected_range_|, splice buffers, preroll buffers, and kConfigChanged signalling. Further, when reading from |selected_range_|, the underlying SourceBufferRange API allows for GetNextConfigId() to peek if there's a config change, but there's not a corresponding peek of the buffer to check that it's non-empty.

tl;dr: this isn't going to be a quick fix that I can produce today. I'll try if I have time next week since I'm build sheriffing the rest of this week.

How about just not allowing zero byte buffers to be inserted?

@#11 That seems simpler. It would preclude hacks (possibly specific to Chrome MSE) where applications might today buffer 0-byte, long-duration keyframes to accomplish coalescing of buffered range gaps. Alternative non-0-byte hacks might suffice still though, for that use case.

I'll work on this simpler alternative (ignore parser buffers emitted containing 0-byte data before sending them to FrameProcessor).

Simpler version needs some test updates still, and also I'm curious if 0-byte text buffers mean anything (likely no). 12 unit tests are failing. I'll pick this up next week.

Hmm. In-band webm text buffers in at least some unit tests are 0-byte data_size, but nonzero side_data_size. (Id and Settings strings are put into side data, content is put into data.)

Since we don't use ffmpeg to decode text, I'll retain 0-byte text in-band. Further, since MSE inband text parsing isn't enabled unless by cmdline parameter (or in our tests), if these 0-byte "content" buffers need skipping, we can do that later.

Interesting. Don't we use text demuxing on Chromecast? So maybe the restriction should be relaxed for text packets?

I reasoned that 0-byte text packets (from looking at the text renderer) are just empty strings, so it didn't seem reasonable to render then.

Or rather it didn't matter if we renderered them or not.

Incomplete, but otherwise working CL (for the simpler alternative) is @ https://chromium-review.googlesource.com/#/c/chromium/src/+/974427 (I want to add media-internals logging of skipped buffers before it lands).

Update CL from #18 is now in CR.

+servolk@: I don't know if Chromecast uses in-band text parsing for src= or MSE (--enable-inband-text-tracks). Do they just use js libs and the HTML5 TrackElement to render text content? I'm also less certain now that the MSE inband WebMWebVTTParser does all the needed things (the format of WebVTT, even in-band WebVTT, may have evolved). matthewjheaney@ was the last active developer of that parser and the bubbling of demuxed text track frames up to Blink IIRC.

Regardless, the CL in #18 exempts 0-byte text packets from being dropped by the MSE side of the pipeline. If same is needed for ffmpeg_demuxer, we should consider doing that, too.

If no one is using --enable-inband-text-tracks we should delete that.

I'm less familiar with WebVTT rendering, but we do indeed appear to propogate settings, id and content (data) to Blink's VTTCue. If content is empty, maybe something in settings (like region) might still impact the visual result -- maybe. This lack of certainty is why I chose to exempt 0-byte data text from being skipped by MSE (also, it let our existing in-band text MSE unit tests continue to pass).

As far as I can see Chromecast isn't using --enable-inband-text-tracks. AFAIK we do indeed use js parsers for subtitle tracks and just use TrackElement for rendering (see https://cs.corp.google.com/piper///depot/google3/chrome/dongle/player/lib/core/captionsmanager.js).

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

commit b5070c03aaef8818fca12a5785893f217ba23d4f
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Tue Mar 27 00:34:45 2018

Remove old test special casing for WebM video BlockGroups

Since r393078, WebMClusterParser no longer inspects payload to
determine "keyframiness" of the blockgroup, this change removes
the special case ChunkDemuxerTest code which fed a valid VP8
keyframe or nonkeyframe payload into generated BlockGroups for
the parser.

This simplifies an upcoming change that allows testing around
0-byte payloads (related to crbug 823375).

BUG= 823375 
TEST=media_unittests passes

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I05b70f4dc22b9f75b7761188ecee9bd4f7909ac8
Reviewed-on: https://chromium-review.googlesource.com/981458
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545905}
[modify] https://crrev.com/b5070c03aaef8818fca12a5785893f217ba23d4f/media/filters/chunk_demuxer_unittest.cc

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

commit 491a2c2c94dbed6c4d65164918ba7f1d6be0a44d
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Tue Mar 27 17:59:39 2018

MSE: Drop empty AV buffers emitted by Stream Parser

Since FFmpeg decode of 0-byte audio and video buffers is now disallowed,
and FFmpegDemuxer skips enqueuing such packets into its DemuxerStream,
this change does similar for MSE:

Any 0-byte |data| audio or video buffers are dropped during initial
processing in FrameProcessor::ProcessFrames(). No 0-byte |data| text
buffers are dropped, because:
* We don't use FFmpeg to decode in-band MSE text, and
* In-band MSE text buffers' |side_data| may contain meaningful
  information. (This assumption isn't asserted by this change).

Note, this change takes the simpler route of dropping 0-byte AV buffers
before they're buffered by the coded frame processing algorithm. A more
complex alternative to skip them when reading from SourceBufferStream
was rejected in hopes this simpler change is sufficient.

Includes ability in ChunkDemuxerTest to vary |block_size_| instead of
always using |kBlockSize|. While the product change is closer to
FrameProcessor, FrameProcessorTests always encode the coded frame's
timestamp as the coded frame contents to enable precise verification of
processing results, so the test of this change is done via
ChunkDemuxerTest instead.

BUG= 823375 ,663438
TEST=ChunkDemuxerTest.ZeroLengthFramesDropped

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ie6a802ba64a576f0219ca7c400fc7dec3a45fbf6
Reviewed-on: https://chromium-review.googlesource.com/974427
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546159}
[modify] https://crrev.com/491a2c2c94dbed6c4d65164918ba7f1d6be0a44d/media/base/test_helpers.h
[modify] https://crrev.com/491a2c2c94dbed6c4d65164918ba7f1d6be0a44d/media/filters/chunk_demuxer_unittest.cc
[modify] https://crrev.com/491a2c2c94dbed6c4d65164918ba7f1d6be0a44d/media/filters/frame_processor.cc
[modify] https://crrev.com/491a2c2c94dbed6c4d65164918ba7f1d6be0a44d/media/filters/frame_processor.h
[modify] https://crrev.com/491a2c2c94dbed6c4d65164918ba7f1d6be0a44d/media/formats/webm/cluster_builder.cc

#25 should fix this. Pending CF verification.

@#21,23: bug 826419 tracks possibly removing inband text track processing.

ClusterFuzz has detected this issue as fixed in range 546148:546167.

Detailed report: https://clusterfuzz.com/testcase?key=5974274821849088

Fuzzer: libFuzzer_mediasource_WEBM_VP8_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  packet.size > 0 in ffmpeg_video_decoder.cc
  media::FFmpegVideoDecoder::FFmpegDecode
  media::FFmpegVideoDecoder::Decode
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=543846:543855
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=546148:546167

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5974274821849088

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 5974274821849088 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

ClusterFuzz has detected this issue as fixed in range 546148:546167.

Detailed report: https://clusterfuzz.com/testcase?key=5974274821849088

Fuzzer: libFuzzer_mediasource_WEBM_VP8_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  packet.size > 0 in ffmpeg_video_decoder.cc
  media::FFmpegVideoDecoder::FFmpegDecode
  media::FFmpegVideoDecoder::Decode
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=543846:543855
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=546148:546167

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5974274821849088

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.