Commit 9a4096be authored by Zhenyao Mo's avatar Zhenyao Mo Committed by Commit Bot

Revert "MSE: Log when MP4 keyframe-ness mismatches the compressed media"

This reverts commit 61697367.

Reason for revert: crbug.com/867520

Original change's description:
> MSE: Log when MP4 keyframe-ness mismatches the compressed media
> 
> Logs to chrome://media-internals when the container's keyframe metadata
> for a frame mismatches the contents of that frame (if the latter was
> determined by the respective bitstream converter's Analyze()
> implementation, of whom only AVC currently does any keyframe analysis).
> 
> BUG=860420,584384
> TEST=Repro crbug 860420 shows logs, new MP4StreamParserTest.AVC_Key* and media
> 
> Change-Id: Ic87eab8726ad76b36b569df08ab2212b267008c2
> Reviewed-on: https://chromium-review.googlesource.com/1149153
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#577751}

TBR=wolenetz@chromium.org,sandersd@chromium.org

Change-Id: I8437af99f24bf04910ad56931a9e91f960a59469
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 860420, 584384
Reviewed-on: https://chromium-review.googlesource.com/1150243Reviewed-by: default avatarZhenyao Mo <zmo@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577968}
parent 352492fd
......@@ -41,7 +41,6 @@ namespace {
const int kMaxEmptySampleLogs = 20;
const int kMaxInvalidConversionLogs = 20;
const int kMaxVideoKeyframeMismatchLogs = 10;
// Caller should be prepared to handle return of Unencrypted() in case of
// unsupported scheme.
......@@ -91,8 +90,8 @@ MP4StreamParser::MP4StreamParser(const std::set<int>& audio_object_types,
has_sbr_(has_sbr),
has_flac_(has_flac),
num_empty_samples_skipped_(0),
num_invalid_conversions_(0),
num_video_keyframe_mismatches_(0) {}
num_invalid_conversions_(0) {
}
MP4StreamParser::~MP4StreamParser() = default;
......@@ -815,19 +814,9 @@ ParseResult MP4StreamParser::EnqueueSample(BufferQueueMap* buffers) {
<< "Prepared video sample is not conformant";
}
// Use |analysis.is_keyframe|, if it was actually determined, for logging
// if the analysis mismatches the container's keyframe metadata for
// |frame_buf|.
if (analysis.is_keyframe.has_value() &&
runs_->is_keyframe() != analysis.is_keyframe.value()) {
LIMITED_MEDIA_LOG(DEBUG, media_log_, num_video_keyframe_mismatches_,
kMaxVideoKeyframeMismatchLogs)
<< "ISO-BMFF container metadata for video frame indicates that the "
"frame is "
<< (runs_->is_keyframe() ? "" : "not ")
<< "a keyframe, but the video frame contents indicate the "
"opposite.";
}
// TODO(wolenetz): Use |analysis.is_keyframe|, if it was actually
// performed, for at least logging if the result mismatches container's
// keyframe metadata for |frame_buf|.
}
}
......
......@@ -148,9 +148,6 @@ class MEDIA_EXPORT MP4StreamParser : public StreamParser {
// Tracks the number of MEDIA_LOGS for invalid bitstream conversion.
int num_invalid_conversions_;
// Tracks the number of MEDIA_LOGS for video keyframe MP4<->frame mismatch.
int num_video_keyframe_mismatches_;
DISALLOW_COPY_AND_ASSIGN(MP4StreamParser);
};
......
......@@ -50,11 +50,7 @@ MATCHER(SampleEncryptionInfoUnavailableLog, "") {
}
MATCHER_P(ErrorLog, error_string, "") {
return CONTAINS_STRING(arg, error_string) && CONTAINS_STRING(arg, "error");
}
MATCHER_P(DebugLog, debug_string, "") {
return CONTAINS_STRING(arg, debug_string) && CONTAINS_STRING(arg, "debug");
return CONTAINS_STRING(arg, error_string);
}
class MP4StreamParserTest : public testing::Test {
......@@ -291,51 +287,6 @@ TEST_F(MP4StreamParserTest, UnknownDuration_V0_AllBitsSet) {
512);
}
TEST_F(MP4StreamParserTest, AVC_KeyAndNonKeyframeness_Match_Container) {
// Both AVC video frames' keyframe-ness metadata matches the MP4:
// Frame 0: AVC IDR, trun.first_sample_flags: sync sample that doesn't
// depend on others.
// Frame 1: AVC Non-IDR, tfhd.default_sample_flags: not sync sample, depends
// on others.
// This is the base case; see also the "Mismatches" cases, below.
auto params = GetDefaultInitParametersExpectations();
params.detected_audio_track_count = 0;
InitializeParserWithInitParametersExpectations(params);
ParseMP4File("bear-640x360-v-2frames_frag.mp4", 512);
}
TEST_F(MP4StreamParserTest, AVC_Keyframeness_Mismatches_Container) {
// The first AVC video frame's keyframe-ness metadata matches the MP4:
// Frame 0: AVC IDR, trun.first_sample_flags: NOT sync sample, DEPENDS on
// others.
// Frame 1: AVC Non-IDR, tfhd.default_sample_flags: not sync sample, depends
// on others.
auto params = GetDefaultInitParametersExpectations();
params.detected_audio_track_count = 0;
InitializeParserWithInitParametersExpectations(params);
EXPECT_MEDIA_LOG(DebugLog(
"ISO-BMFF container metadata for video frame indicates that the frame is "
"not a keyframe, but the video frame contents indicate the opposite."));
ParseMP4File("bear-640x360-v-2frames-keyframe-is-non-sync-sample_frag.mp4",
512);
}
TEST_F(MP4StreamParserTest, AVC_NonKeyframeness_Mismatches_Container) {
// The second AVC video frame's keyframe-ness metadata matches the MP4:
// Frame 0: AVC IDR, trun.first_sample_flags: sync sample that doesn't
// depend on others.
// Frame 1: AVC Non-IDR, tfhd.default_sample_flags: SYNC sample, DOES NOT
// depend on others.
auto params = GetDefaultInitParametersExpectations();
params.detected_audio_track_count = 0;
InitializeParserWithInitParametersExpectations(params);
EXPECT_MEDIA_LOG(DebugLog(
"ISO-BMFF container metadata for video frame indicates that the frame is "
"a keyframe, but the video frame contents indicate the opposite."));
ParseMP4File("bear-640x360-v-2frames-nonkeyframe-is-sync-sample_frag.mp4",
512);
}
TEST_F(MP4StreamParserTest, MPEG2_AAC_LC) {
InSequence s;
std::set<int> audio_object_types;
......
......@@ -74,20 +74,6 @@ vorbis-packet-1 - timestamp: 0ms, duration: 0ms
vorbis-packet-2 - timestamp: 0ms, duration: 0ms
vorbis-packet-3 - timestamp: 2902ms, duration: 0ms
// MSE MP4 keyframe-metadata versus encoded AVC keyframe-ness test media:
bear-640x360-v-2frames_frag.mp4 - Just first 2 video frames of bear-640x360-v_frag.mp4, created with:
ffmpeg -i bear-640x360-v_frag.mp4 -vcodec copy -movflags frag_keyframe+empty_moov+default_base_moof \
-vframes 2 bear-640x360-v-2frames_frag.mp4
It's 1 keyframe + 1 non-keyframe, with container's frame keyframe-ness correct.
bear-640x360-v-2frames-keyframe-is-non-sync-sample_frag.mp4
This is bear-640x360-v-2frames_frag.mp4, with manually updated trun.first_sample_flags:
s/0x02000000/0x01010000 (first frame is non-sync-sample, depends on another
frame, mismatches compressed h264 first frame's keyframe-ness).
bear-640x360-v-2frames-nonkeyframe-is-sync-sample_frag.mp4
This is bear-640x360-v-2frames_frag.mp4, with manually updated tfhd.default_sample_flags:
s/0x01010000/0x02000000 (second frame is sync-sample, doesn't depend on other
frames, mismatches compressed h264 second frame's nonkeyframe-ness).
// 10-bit test file(s)
bear-320x180-hi10p.mp4
bear-320x240-vp9_profile2.webm - VP9 encoded video with profile 2 (10-bit, 4:2:0). Codec string: vp09.02.10.10.01.02.02.02.00.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment