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

Issue 2928043002: media: Pass output_cb to MojoMediaClient::CreateVideoDecoder() (Closed)

Created:
3 years, 6 months ago by sandersd (OOO until July 31)
Modified:
3 years, 6 months ago
Reviewers:
watk, xhwang
CC:
chromium-reviews, feature-media-reviews_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, posciak+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, alokp+watch_chromium.org, darin (slow to review)
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

media: Pass output_cb to MojoMediaClient::CreateVideoDecoder() This callback allows us to associate a ReleaseMailboxCB with each output, without having to implement a remote interface for VideoFrames. This exactly matches the semantics of VDA::ReusePictureBuffer, and will be used to implement it, but also provides a place to stash TextureRef objects for other decoders. BUG=522298 Review-Url: https://codereview.chromium.org/2928043002 Cr-Commit-Position: refs/heads/master@{#479890} Committed: https://chromium.googlesource.com/chromium/src/+/174520a0999989f73c06047b5ce627ad20425489

Patch Set 1 #

Total comments: 6

Patch Set 2 : Address nits. #

Patch Set 3 : Rebase. #

Patch Set 4 : Code clarity. #

Patch Set 5 : Tag with bug. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+54 lines, -12 lines) Patch
M media/mojo/services/gpu_mojo_media_client.h View 1 chunk +2 lines, -1 line 0 comments Download
M media/mojo/services/gpu_mojo_media_client.cc View 1 chunk +2 lines, -1 line 0 comments Download
M media/mojo/services/mojo_media_client.h View 1 2 3 4 4 chunks +16 lines, -1 line 0 comments Download
M media/mojo/services/mojo_media_client.cc View 1 chunk +2 lines, -1 line 0 comments Download
M media/mojo/services/mojo_video_decoder_service.h View 1 2 chunks +7 lines, -1 line 0 comments Download
M media/mojo/services/mojo_video_decoder_service.cc View 1 2 3 4 chunks +25 lines, -7 lines 0 comments Download

Messages

Total messages: 22 (11 generated)
sandersd (OOO until July 31)
3 years, 6 months ago (2017-06-08 23:06:32 UTC) #2
watk
lgtm with nits https://codereview.chromium.org/2928043002/diff/1/media/mojo/services/mojo_media_client.h File media/mojo/services/mojo_media_client.h (right): https://codereview.chromium.org/2928043002/diff/1/media/mojo/services/mojo_media_client.h#newcode44 media/mojo/services/mojo_media_client.h:44: // Same as VideoFrame::ReleaseMailboxCB, but it ...
3 years, 6 months ago (2017-06-09 00:35:44 UTC) #3
sandersd (OOO until July 31)
https://codereview.chromium.org/2928043002/diff/1/media/mojo/services/mojo_media_client.h File media/mojo/services/mojo_media_client.h (right): https://codereview.chromium.org/2928043002/diff/1/media/mojo/services/mojo_media_client.h#newcode44 media/mojo/services/mojo_media_client.h:44: // Same as VideoFrame::ReleaseMailboxCB, but it doesn't have to ...
3 years, 6 months ago (2017-06-09 00:59:20 UTC) #4
xhwang
Sorry for the delay. I am looking at it and will post comments tonight or ...
3 years, 6 months ago (2017-06-13 00:35:58 UTC) #7
xhwang
Quick comment before we go into details. Have you considered to use a unified FrameResourceReleaser ...
3 years, 6 months ago (2017-06-13 23:23:37 UTC) #8
sandersd (OOO until July 31)
On 2017/06/13 23:23:37, xhwang wrote: > Quick comment before we go into details. Have you ...
3 years, 6 months ago (2017-06-13 23:32:54 UTC) #9
xhwang
On 2017/06/13 23:32:54, sandersd wrote: > On 2017/06/13 23:23:37, xhwang wrote: > > Quick comment ...
3 years, 6 months ago (2017-06-13 23:39:47 UTC) #11
sandersd (OOO until July 31)
On 2017/06/13 23:32:54, sandersd wrote: > On 2017/06/13 23:23:37, xhwang wrote: > > Quick comment ...
3 years, 6 months ago (2017-06-13 23:43:47 UTC) #12
xhwang
As discussed offline, it'll be nice to merge this path with the current FrameResourceReleaser (recommended ...
3 years, 6 months ago (2017-06-14 00:21:59 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2928043002/80001
3 years, 6 months ago (2017-06-15 21:39:54 UTC) #19
commit-bot: I haz the power
3 years, 6 months ago (2017-06-16 01:47:03 UTC) #22
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/174520a0999989f73c06047b5ce6...

Powered by Google App Engine
This is Rietveld 408576698