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

Issue 518163003: media: scaffolding and plumbing for MojoRenderer{Impl, Service} (Closed)

Created:
6 years, 3 months ago by tim (not reviewing)
Modified:
6 years, 3 months ago
Reviewers:
CC:
chromium-reviews, qsr+mojo_chromium.org, Aaron Boodman, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, feature-media-reviews_chromium.org, darin (slow to review), ben+mojo_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@medr
Project:
chromium
Visibility:
Public.

Description

media: scaffolding and plumbing for MojoRenderer{Impl, Service} * Add skeleton client-side and service-side MediaRenderer pieces with basic plumbing and connections but no real logic. * model a media::DemuxerStream in a mojom to clean up MediaRenderer interface and line-up with RendererImpl expectations. * add DecoderConfig plumbing support * add a basic unittest verifying things do actually hook up * consolidate around 'media_mojo_' naming in media.gyp and add media_mojo target to build all this stuff. BUG=410451

Patch Set 1 : #

Total comments: 4

Patch Set 2 : #

Patch Set 3 : add test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1357 lines, -80 lines) Patch
M media/media.gyp View 1 2 2 chunks +119 lines, -56 lines 0 comments Download
A media/mojo/interfaces/demuxer_stream.mojom View 1 2 1 chunk +54 lines, -0 lines 0 comments Download
M media/mojo/interfaces/media_renderer.mojom View 1 2 1 chunk +3 lines, -10 lines 0 comments Download
M media/mojo/interfaces/media_types.mojom View 1 2 1 chunk +87 lines, -7 lines 0 comments Download
M media/mojo/services/media_type_converters.h View 1 2 chunks +10 lines, -0 lines 0 comments Download
M media/mojo/services/media_type_converters.cc View 1 2 2 chunks +138 lines, -6 lines 0 comments Download
M media/mojo/services/media_type_converters_unittest.cc View 1 3 chunks +15 lines, -1 line 0 comments Download
A media/mojo/services/mojo_demuxer_stream_adapter.h View 1 2 1 chunk +76 lines, -0 lines 0 comments Download
A media/mojo/services/mojo_demuxer_stream_adapter.cc View 1 2 1 chunk +102 lines, -0 lines 0 comments Download
A media/mojo/services/mojo_demuxer_stream_impl.h View 1 1 chunk +53 lines, -0 lines 0 comments Download
A media/mojo/services/mojo_demuxer_stream_impl.cc View 1 1 chunk +60 lines, -0 lines 0 comments Download
A media/mojo/services/mojo_renderer_impl.h View 1 1 chunk +87 lines, -0 lines 0 comments Download
A media/mojo/services/mojo_renderer_impl.cc View 1 1 chunk +119 lines, -0 lines 0 comments Download
A media/mojo/services/mojo_renderer_service.h View 1 2 1 chunk +89 lines, -0 lines 0 comments Download
A media/mojo/services/mojo_renderer_service.cc View 1 2 1 chunk +154 lines, -0 lines 0 comments Download
A media/mojo/services/renderer_unittest.cc View 1 2 1 chunk +191 lines, -0 lines 0 comments Download

Messages

Total messages: 22 (20 generated)
xhwang
some preliminary comments I had when I looked at this CL... https://codereview.chromium.org/518163003/diff/40001/media/mojo/services/mojo_renderer_client_impl.h File media/mojo/services/mojo_renderer_client_impl.h (right): ...
6 years, 3 months ago (2014-09-02 17:05:53 UTC) #1
tim (not reviewing)
6 years, 3 months ago (2014-09-02 18:20:05 UTC) #2
https://codereview.chromium.org/518163003/diff/40001/media/mojo/services/mojo...
File media/mojo/services/mojo_renderer_client_impl.h (right):

https://codereview.chromium.org/518163003/diff/40001/media/mojo/services/mojo...
media/mojo/services/mojo_renderer_client_impl.h:16: class MojoRendererClientImpl
: public Renderer,
On 2014/09/02 17:05:52, xhwang wrote:
> From media pipeline's perspective, this is a mojo based Renderer
implementation.
> The fact that it's also a mojo::MediaRendererClient (hence the "Client") is an
> implementation detail. How about s/MojoRendererClientImpl/MojoRendererImpl?

Yeah, I had it named that way before... I think the reason I changed it is
because the "mojo based renderer implementation" is actually two-fold: it has a
client and a service.  Although I didn't make this really obvious as the two
names aren't symmetric.  What about if we had 

MojoRendererClientImpl / MojoRendererServiceImpl
or
MojoRendererImplClient / MojoRendererImplService

?

https://codereview.chromium.org/518163003/diff/40001/media/mojo/services/mojo...
media/mojo/services/mojo_renderer_client_impl.h:22: mojo::ServiceProvider*
media_renderer_provider);
On 2014/09/02 17:05:53, xhwang wrote:
> This can also take a DemuxerStreamProvider* demuxer_stream_provider in the
> future.
> 
> See https://codereview.chromium.org/523283002/

cool!

Powered by Google App Engine
This is Rietveld 408576698