fdoray@ could you please look into this.please feel free to re-assigned back if needed. thanks in advance !

My CL is supposed to be a no-op https://chromium.googlesource.com/chromium/src/+/f920dcf7ef1f0cd43ffcada2918d23d41f1c8428 Re-assigning to xhwang@chromium.org who might know more about the media pipeline than me.

+liberato@ who recently changed some of this code.

dalecurtis: Which CL from liberato@ are you referring to? I didn't see anything obvious from the blamelist.

He changed pipeline integration tests recently. Don't have the CL on hand.

i think he's referring to https://codereview.chromium.org/2479633002/

The issue is found on r431874 which does include liberato's change (r429690).

Since I don't see anything related to media in the blamelist, I'll assume the list is off.

Assigning to liberato so we can start from his change.

i've reproduced it at ToT with bc29809b850a2dcd2f803d4f25238085f64d7906 (Fix pipeline integration tests for seek / duration changes.) reverted.

will poke at it some more to see what's up.

looks like libavformat::parse_packet is looping forever, not consuming any of the packet.

Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

dale: this exists in ffmpeg too.  who's the right person to contact @ffmpeg?

in particular: running this hangs:
ffmpeg -i ./fuzz_repro_case ./out.flac

ffmpeg -v
ffmpeg version N-82613-g3ee5993 Copyright (c) 2000-2016 the FFmpeg developers
  built with gcc 4.8 (Ubuntu 4.8.4-2ubuntu1~14.04.3)

(i.e.,origin/master in ffmpeg's repo)

michael@niedermayer.cc is the right contact. Feel free to cc me on it. You'll want to include the test case when you e-mail him.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/7e5307d753a5a21f6d02663ccccf2acdf7aeae0e

commit 7e5307d753a5a21f6d02663ccccf2acdf7aeae0e
Author: liberato@chromium.org <liberato@chromium.org>
Date: Mon Nov 28 21:48:38 2016

Consume headers in flac parser.

Fix from Michael Neidermayer.  Prevents clusterfuzz test case from
looping forever trying to find the next header.

BUG= 665305 
Change-Id: If518327c93569c475bdabec154ac5c4499b74acd
Reviewed-on: https://chromium-review.googlesource.com/414310
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>

[modify] https://crrev.com/7e5307d753a5a21f6d02663ccccf2acdf7aeae0e/libavcodec/flac_parser.c
[modify] https://crrev.com/7e5307d753a5a21f6d02663ccccf2acdf7aeae0e/chromium/patches/README

liberato@: Was #14 really a cherry-pick from upstream ffmpeg repo? If so, what's the upstream hash (and for future, just the hash would need to go into top of chromium/patches/README)? If not a cherry-pick from upstream, 'sallgood :)

Also, is this something needed in M56, once the ffmpeg roll ( issue 591845 ) merges to M56? If so, mark this as blocking 591845 (yeah, a bit weird, but that's how I've marked the others), please request the merge and make it happen if approved. Thanks :)

From chat w/liberato: #14 was not a cherry-pick, but upstream will shortly have same.

w.r.t. M56- still pending an answer.

Further from chat w/liberato: M56 could use this fix too, to prevent CF (and potentially Chrome) from hanging in similar scenario on M56 branch. I'll request and do the merge to M56 if approved, since I'm doing a bunch of other similar ones.

Currently, requesting the merge is pending CF verifying this issue is fixed in trunk.

(also pending https://codereview.chromium.org/2539573003/ landing in trunk :) )

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

commit 535eedb7eb8b72d6aaf89ff7fdf3b346c06e15ef
Author: liberato <liberato@chromium.org>
Date: Tue Nov 29 22:05:42 2016

$ git log d16162e3f..7e5307d75 --date=short --no-merges --format='%ad %ae %s'
2016-11-28 liberato@chromium.org Consume headers in flac parser.

BUG= 665305 

Review-Url: https://codereview.chromium.org/2539573003
Cr-Commit-Position: refs/heads/master@{#435087}

[modify] https://crrev.com/535eedb7eb8b72d6aaf89ff7fdf3b346c06e15ef/DEPS

thank wolenetz@ for merging this back!

i'll add a unit test to ToT for this case.  please re-assign to me when you're done with the merge so i don't forget.  :)

ClusterFuzz has detected this issue as fixed in range 435044:435100.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6539465347825664

Fuzzer: libfuzzer_media_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: Hang
Crash Address: 
Crash State:
  media_pipeline_integration_fuzzer
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=413142:413297
Fixed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=435044:435100

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv94PfstBeAYqHILn6ENyjePcjYSywUyO5UgTo4apdFzCdPPmyPS5bJPjWJD82f2PqWjKY-4JQYjQiVm4hjiXQtcEUrQC2YUu_GhVJj0oEqHlQN7KdRG2CJ-uX4yB6X4YQXbaKxiJoj-8sbkUdRKCx0kQR0rzNw?testcase_id=6539465347825664


See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.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 is verified as fixed, closing issue.

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

Re-opening to track #20 getting done later.
Requesting merge to M56 of #19 (which will require a buildspec change in branch 2924, so cc+=amineer@ for assistance with that eventuality).

[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.

We should probably get updating branch DEPS into a better state rather than having me process all of them.

+mmoss@ / dimu@ - if they don't exist, can we please get instructions written / posted for developers on how to roll branch DEPS in the interim until issue 661382 is fixed, and have them get pointed out specifically in the commit sentry message that is sent when it detects a DEPS change?  I've been doing this manually for people for a while but I've run out of steam.

Approving for merge into 56 (build 2924)

I have instructions for how to do buildspec updates from mmoss@ (thank you!) -- simpler than I thought they might be. I'll proceed with the src.git DEPS branch 2924 update (no-op other than keeping that branches' src.git DEPS clean) along with the for-realz branch 2924 buildspec update to complete the merge to M56 of #19.

@#27 Per mmoss@, no need to update src.git DEPS-only for these kinds of merges:
"no, not much point in that unless everybody does it, which they don't
it's still going to be broken, and then might even just be confusing if some updates are missing while others are there
probably best to leave it untouched"

tl;dr: merge to M56 of fix (DEPS roll like #19) will just be a buildspec update.

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/4648212cb93c76c8deae2f6143b892c0d3e79787

commit 4648212cb93c76c8deae2f6143b892c0d3e79787
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Thu Dec 01 21:10:18 2016

The fix is now merged to M56 (as of #29). Updating labels, since buildspec updates don't seem to do that.

Also:
Per #20, => liberato@ for adding unit tests to cover the fix (in at least M57 please)

Taking this on with liberato@'s help

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

commit 0b2581ebf78734be17d52ea9917bf8441fc211e1
Author: crouleau <crouleau@chromium.org>
Date: Fri Dec 02 23:56:54 2016

Add ffmpeg regression test for flac file hang found by clusterfuzz.

Added e9ff58f7acde29c4e53c761da775293e776bcc23 to internal repo.

BUG= 665305 

Review-Url: https://codereview.chromium.org/2547203002
Cr-Commit-Position: refs/heads/master@{#436085}

[modify] https://crrev.com/0b2581ebf78734be17d52ea9917bf8441fc211e1/media/ffmpeg/ffmpeg_regression_tests.cc

The fix for the hang now triggers a DCHECK failure (https://cs.chromium.org/chromium/src/media/filters/ffmpeg_demuxer.cc?rcl=0&l=557) when running Cr665305/FFmpegRegressionTest.BasicPlayback/0. I got this on current trunk, under asan tooling, on linux.

See also  issue 671071 ; which I was about to look at.

(Note, I use dcheck_always_on = true in my asan args.gn, so even though is_debug = false, DCHECK failures like that in #34 can get caught earlier.)

interesting... when i run it, i get a different failure:

[ RUN      ] Cr665305/FFmpegRegressionTest.BasicPlayback/0
../../media/ffmpeg/ffmpeg_regression_tests.cc:358: Failure
Value of: WaitUntilEndedOrError()
  Actual: 0
Expected: GetParam().end_status
Which is: 3
[  FAILED  ] Cr665305/FFmpegRegressionTest.BasicPlayback/0, where GetParam() = 16-byte object <18-F8 90-84 CC-17 00-00 00-00 00-00 03-00 00-00> (3 ms)

Possibly just a flaky type test, we have a fair number of those in that file.

@ #37, did you use dcheck_always_on = true in your args.gn that you built from? I didn't try without that, so I don't know if it eventually matches or flakes vs the expectation, since the DCHECK was hit first.

i did.

@ #40: Maybe try --gtest_repeat=100
FWIW, on linux repro that hit the DCHECK, my args.gn was:

dcheck_always_on = true
is_debug = false
is_asan = true
enable_nacl = false
use_goma = true
ffmpeg_branding = "ChromeOS"
proprietary_codecs = true
enable_mse_mpeg2ts_stream_parser = true
enable_hevc_demuxing = true

i've got a repro now.  my local copy of the media repo had the wrong input file -- i probably overwrote it accidentally.

The fix for this bug will also likely impact/fix  crbug.com/656763 , if the fix involves handling buffers with kNoTimestamp.

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

commit 722dfe42527ed575678e5145ef3e58f03013030a
Author: liberato <liberato@chromium.org>
Date: Tue Dec 20 23:49:57 2016

Fix up missing timestamps in FFmpegDemuxer.

Previously, we would DCHECK if FFmpeg provded no timestamp for a
buffer.  We now replace this with zero for the first packet, and
with an advance over the previous buffer for later ones.

The rationale is that bad media might cause this, and we previously
had a DCHECK to catch it.  This makes the behavior consistent even
in releaes builds.

BUG= 665305 ,  673079 
TEST=ffmpeg_regression_tests

Review-Url: https://codereview.chromium.org/2563183002
Cr-Commit-Position: refs/heads/master@{#439933}

[modify] https://crrev.com/722dfe42527ed575678e5145ef3e58f03013030a/media/ffmpeg/ffmpeg_regression_tests.cc
[modify] https://crrev.com/722dfe42527ed575678e5145ef3e58f03013030a/media/filters/ffmpeg_demuxer.cc
[modify] https://crrev.com/722dfe42527ed575678e5145ef3e58f03013030a/media/filters/ffmpeg_demuxer.h

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

commit 067e03ca0fc3a4f64fc5f324d665e5c10268cd14
Author: tguilbert <tguilbert@chromium.org>
Date: Sat Jan 14 02:14:25 2017

Fix int-overflow due to absent stream timestamp

 crbug.com/665305  introduced some fixes for buffers with no timestamps.
However, if we did not receive a valid stream timestamp from the
demuxer, the first buffer that we output can still end up having no
timestamp (which eventually leads to an integer overflow in
VideoRendererAlgorithm).

This CL fixes the issue by reporting a demuxer error, rather than
emitting a buffer with no timestamp and relying on the decoder to error
out.

BUG= 665305 ,  656763 
TEST= fixes media_pipeline_integration_fuzzer. Ran ffmpeg_integration_tests as ASAN/MSAN/UBSAN. Ran Media UTs

Review-Url: https://codereview.chromium.org/2635573002
Cr-Commit-Position: refs/heads/master@{#443769}

[modify] https://crrev.com/067e03ca0fc3a4f64fc5f324d665e5c10268cd14/media/ffmpeg/ffmpeg_regression_tests.cc
[modify] https://crrev.com/067e03ca0fc3a4f64fc5f324d665e5c10268cd14/media/filters/ffmpeg_demuxer.cc

CF case is reported as fixed, so closing this. We were worried this might impact parse errors. I audited our pipeline status histograms and the only one that changed is the AudioOnly one: 

https://uma.googleplex.com/p/chrome/timeline_v2/?sid=77e08b765350d818fb99b7d1726796f1

It hasn't hit stable, but I think the level of errors is acceptable.