I'm optimistic this may not be too risky; the Chromecast MSE mp2t_stream_parser already relies on layer violation (peeking at h264 headers' NALU IDRSlice presence) to determine avc keyframe-ness.

Do we want to do this though? Doesn't this just perpetuate bad media?

Great question, Dale. Sites do need to fix their streams.

This bug's existence shouldn't be seen by sites as a reason to delay fixing their streams.

Sites have had a long runway (at least M68 Dev/Canary, M69 Dev/Canary/Beta) for conforming and detecting regressions caused by these streams' processing in MseBufferByPts.  Just because some other implementations may peek cross-layer to correct the required MSE bytestream metadata doesn't mean we should, too. Neither does it mean such a change would be risk-free for our implementation.

So on further thought, closing this as WontFix.

Re-activating: In addition to a couple known sites that feed bad ISOBMFF keyframe-ness for AVC to Chrome MSE, this may also fix a long tail of other sites experiencing increased anomalies with MseBufferByPts. Increase in non-Mac/Linux of !PIPELINE_OK with MseBufferByPts is consistent with Win/CrOS/Android increased (H/W avc) decoder strictness, once initialized (from chat w/sandersd@ today).

CL https://chromium-review.googlesource.com/c/chromium/src/+/1205640 should fix this for AVC-in-MP4 in Chrome MSE.

This will likely need to merge to M70 once it's baked in ToT/M71. It can't, by itself, go to M-69, since the change depends on mp4-avc keyframe analysis code that landed in M70.

Further to #4, I also verified playback with Edge, Safari and Firefox: they play without decode anomaly the samples from the couple known sites with bad ISOBMFF keyframe-ness.

 Issue 844799  has been merged into this issue.

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

commit 1240638ead7a9b3568b4a30918aff44611c00208
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Wed Sep 05 21:40:56 2018

MSE: Use video (AVC) keyframeness when it differs from MP4's

Due to prevalence of MSE support in other browsers for legacy
incorrectly muxed content where AVC keyframeness is incorrectly marked
in the MP4 container, this change expands the previous
chrome://media-internals logging when such mismatch is detected, to
trust the keyframe-ness of the AVC coded frame for buffering and
decoding.  Otherwise, such streams, especially those with frames
incorrectly marked as keyframes in the MP4, would be buffered as
out-of-order GOPS rather than out-of-order nonkeyframes, corrupting the
buffering and decode sequence on playback in the new, compliant,
MseBufferByPts logic.  Seek preroll from actual AVC nonkeyframes in
legacy MseBufferByDts logic should also be fixed by this change.

BUG= 879734 , 844799 , 229412 ,760264

Change-Id: I90f53ec333e1aeeeaf39e96e176438cb13b4a195
Reviewed-on: https://chromium-review.googlesource.com/1205640
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589009}
[modify] https://crrev.com/1240638ead7a9b3568b4a30918aff44611c00208/media/formats/mp4/mp4_stream_parser.cc
[modify] https://crrev.com/1240638ead7a9b3568b4a30918aff44611c00208/media/formats/mp4/mp4_stream_parser_unittest.cc

CC+abdulsyed@
I'll likely request merge-to-m70 tomorrow once #7 has had >= 24 hours bake in Canary; its first Canary was 71.0.3544.0.

I'll also check on experiment data for MseBufferByPts with this fix in Canary M71 once there is data to check (probably sometime tomorrow), to further help justify merging this to M70.

Experiment data for MseBufferByPts since #7 in M71 shows no statistically significant regression versus before #7, and looks like it (at larger scale, e.g. M70 beta) may provide an improvement for PIPELINE_STATUS for H/W 264 stream decodes in MSE.
Follow-up analysis of increase of CHUNK_DEMUXER_ERROR_APPEND_FAILED (existed before and after #7) will still be needed once experiment data following the merge of #7 to M70/Beta is available.

Please apply affected OSs.

Also, this fix is confirmed to fix specific sites' media playback with MseBufferByPts. For those sites' media, playback is severely broken without this fix (which allows such less conformant media to work with MseBufferByPts).

branch:3538

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

commit fc67c41a328c4564c3e39d087b1510b28b32abce
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Wed Sep 12 18:54:47 2018

To M70: MSE: Use video (AVC) keyframeness when it differs from MP4's

Due to prevalence of MSE support in other browsers for legacy
incorrectly muxed content where AVC keyframeness is incorrectly marked
in the MP4 container, this change expands the previous
chrome://media-internals logging when such mismatch is detected, to
trust the keyframe-ness of the AVC coded frame for buffering and
decoding.  Otherwise, such streams, especially those with frames
incorrectly marked as keyframes in the MP4, would be buffered as
out-of-order GOPS rather than out-of-order nonkeyframes, corrupting the
buffering and decode sequence on playback in the new, compliant,
MseBufferByPts logic.  Seek preroll from actual AVC nonkeyframes in
legacy MseBufferByDts logic should also be fixed by this change.

BUG= 879734 , 844799 , 229412 ,760264
TBR=dalecurtis@chromium.org,sandersd@chromium.org

Change-Id: I90f53ec333e1aeeeaf39e96e176438cb13b4a195
Reviewed-on: https://chromium-review.googlesource.com/1205640
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#589009}(cherry picked from commit 1240638ead7a9b3568b4a30918aff44611c00208)
Reviewed-on: https://chromium-review.googlesource.com/1222197
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#337}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/fc67c41a328c4564c3e39d087b1510b28b32abce/media/formats/mp4/mp4_stream_parser.cc
[modify] https://crrev.com/fc67c41a328c4564c3e39d087b1510b28b32abce/media/formats/mp4/mp4_stream_parser_unittest.cc

It looks like #14 just barely missed the initial beta M70 releases, but it should come on next M70 beta update (if/when that happens) and when M70 goes stable.