Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(59)

Issue 1292703005: Remove constraint_set_flag checks from ParseH264CodecID(). (Closed)

Created:
2 years, 3 months ago by sandersd (OOO until July 31)
Modified:
2 years, 3 months ago
Reviewers:
ddorwin, wolenetz
CC:
chromium-reviews, feature-media-reviews_chromium.org, jam, darin-cc_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove constraint_set_flag checks from ParseH264CodecID(). The spec does not require these flags to be set; they exist to signal compatability with a profile without explicitly being that profile. BUG=522744 Committed: https://crrev.com/96f574ad73be4ec5b7b8e80f9d16802dc3a537c7 Cr-Commit-Position: refs/heads/master@{#344666}

Patch Set 1 #

Total comments: 9

Patch Set 2 : #

Total comments: 5

Patch Set 3 : #

Patch Set 4 : #

Messages

Total messages: 24 (10 generated)
sandersd (OOO until July 31)
2 years, 3 months ago (2015-08-20 19:13:30 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1292703005/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1292703005/1
2 years, 3 months ago (2015-08-20 19:50:12 UTC) #4
ddorwin
LGTM, but we should see if wolenetz@ has anything to say. https://codereview.chromium.org/1292703005/diff/1/content/browser/media/media_canplaytype_browsertest.cc File content/browser/media/media_canplaytype_browsertest.cc (right): ...
2 years, 3 months ago (2015-08-20 20:18:05 UTC) #6
sandersd (OOO until July 31)
https://codereview.chromium.org/1292703005/diff/1/content/browser/media/media_canplaytype_browsertest.cc File content/browser/media/media_canplaytype_browsertest.cc (right): https://codereview.chromium.org/1292703005/diff/1/content/browser/media/media_canplaytype_browsertest.cc#newcode610 content/browser/media/media_canplaytype_browsertest.cc:610: // The first two digits must be 4D. The ...
2 years, 3 months ago (2015-08-20 20:26:07 UTC) #7
ddorwin
lgtm https://codereview.chromium.org/1292703005/diff/1/content/browser/media/media_canplaytype_browsertest.cc File content/browser/media/media_canplaytype_browsertest.cc (right): https://codereview.chromium.org/1292703005/diff/1/content/browser/media/media_canplaytype_browsertest.cc#newcode614 content/browser/media/media_canplaytype_browsertest.cc:614: EXPECT_EQ(kPropProbably, CanPlay("'video/mp4; codecs=\"avc1.4D001E\"'")); On 2015/08/20 20:26:06, sandersd wrote: ...
2 years, 3 months ago (2015-08-20 20:42:39 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1292703005/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1292703005/40001
2 years, 3 months ago (2015-08-20 21:03:50 UTC) #11
wolenetz
lgtm % 2 nits: https://codereview.chromium.org/1292703005/diff/20001/content/browser/media/media_canplaytype_browsertest.cc File content/browser/media/media_canplaytype_browsertest.cc (right): https://codereview.chromium.org/1292703005/diff/20001/content/browser/media/media_canplaytype_browsertest.cc#newcode597 content/browser/media/media_canplaytype_browsertest.cc:597: // The first two digits ...
2 years, 3 months ago (2015-08-20 21:06:43 UTC) #13
wolenetz
https://codereview.chromium.org/1292703005/diff/20001/media/base/mime_util.cc File media/base/mime_util.cc (right): https://codereview.chromium.org/1292703005/diff/20001/media/base/mime_util.cc#newcode500 media/base/mime_util.cc:500: // avc1.64x0yy - H.264 High On 2015/08/20 21:06:43, wolenetz ...
2 years, 3 months ago (2015-08-20 21:12:38 UTC) #14
sandersd (OOO until July 31)
https://codereview.chromium.org/1292703005/diff/20001/content/browser/media/media_canplaytype_browsertest.cc File content/browser/media/media_canplaytype_browsertest.cc (right): https://codereview.chromium.org/1292703005/diff/20001/content/browser/media/media_canplaytype_browsertest.cc#newcode597 content/browser/media/media_canplaytype_browsertest.cc:597: // The first two digits must be 42. The ...
2 years, 3 months ago (2015-08-20 21:14:01 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1292703005/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1292703005/60001
2 years, 3 months ago (2015-08-20 21:16:32 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/93494)
2 years, 3 months ago (2015-08-20 23:30:42 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1292703005/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1292703005/60001
2 years, 3 months ago (2015-08-21 01:24:00 UTC) #22
commit-bot: I haz the power
Committed patchset #4 (id:60001)
2 years, 3 months ago (2015-08-21 03:00:20 UTC) #23
commit-bot: I haz the power
2 years, 3 months ago (2015-08-21 03:01:03 UTC) #24
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/96f574ad73be4ec5b7b8e80f9d16802dc3a537c7
Cr-Commit-Position: refs/heads/master@{#344666}

Powered by Google App Engine
This is Rietveld efc10ee0f