Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(381)

Issue 1283573002: Do the codec string parsing for MSE in media, instead of Blink (Closed)

Created:
5 years, 4 months ago by servolk
Modified:
5 years, 4 months ago
CC:
blink-reviews, mlamouri+watch-blink_chromium.org, feature-media-reviews_chromium.org, dglazkov+blink, eric.carlson_apple.com, sandersd (OOO until July 31)
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Do the codec string parsing for MSE in media, instead of Blink Other code paths parse the codecs string in media/ code using media::ParseCodecString (see KeySystemConfigSelector::IsSupportedContentType and RendererBlinkPlatformImpl::MimeRegistry::supportsMediaMIMEType). I created a new overload of WebSourceBufferImpl::addSourceBuffer that does the same (https://codereview.chromium.org/1281663003/), now we can remove blink::ContentType::codecs to consolidate all code paths. BUG=none Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200489

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -20 lines) Patch
M Source/modules/mediasource/MediaSource.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/mediasource/MediaSource.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/platform/ContentType.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/platform/ContentType.cpp View 1 chunk +0 lines, -15 lines 0 comments Download
M public/platform/WebMediaSource.h View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 15 (5 generated)
wolenetz
FWIW, this looks to me like a good simplification/deduplication (removing ContentType::codecs() string splitting in favor ...
5 years, 4 months ago (2015-08-12 17:33:01 UTC) #2
servolk
5 years, 4 months ago (2015-08-12 21:33:07 UTC) #5
philipj_slow
lgtm It looks like the other entry point to the shared code path is RendererBlinkPlatformImpl::MimeRegistry::supportsMediaSourceMIMEType, ...
5 years, 4 months ago (2015-08-13 08:19:32 UTC) #6
philipj_slow
Also, despite the path, this is Blink, not WebKit :)
5 years, 4 months ago (2015-08-13 08:19:58 UTC) #7
ddorwin
Please specifically mention "MSE" in the first line of the commit message. Also, s/WebKit/Blink/.
5 years, 4 months ago (2015-08-13 16:59:47 UTC) #9
servolk
On 2015/08/13 08:19:32, philipj wrote: > lgtm > > It looks like the other entry ...
5 years, 4 months ago (2015-08-13 17:26:56 UTC) #10
servolk
On 2015/08/13 16:59:47, ddorwin wrote: > Please specifically mention "MSE" in the first line of ...
5 years, 4 months ago (2015-08-13 17:27:05 UTC) #11
wolenetz
rs lgtm
5 years, 4 months ago (2015-08-13 17:55:21 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1283573002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1283573002/1
5 years, 4 months ago (2015-08-13 18:44:31 UTC) #14
commit-bot: I haz the power
5 years, 4 months ago (2015-08-13 20:29:33 UTC) #15
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200489

Powered by Google App Engine
This is Rietveld 408576698