New issue
Advanced search Search tips

Issue 751838 link

Starred by 2 users
Status: Available
Owner: ----
Cc:
xhw...@chromium.org
dalecur...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 759710


Sign in to add a comment

Use base::OnceCallback in src/media

Project Member Reported by xhw...@chromium.org, Aug 2 2017 
In media, a lot of callbacks are only supposed to be run once, e.g. InitCB, DecodeCB, DecryptCB etc. We should convert them to use OnceCallback to make things easier to reason about.

Also, using OnceCallback would allow us to use ScopedCallbackRunner to have a clean solution making sure the callback will always run.

However, OnceCallback is a move-only type. Currently gmock doesn't support move-only types well so typically we have a workaround in the mock classes. We don't want to use this workaround extensively when we switch to use OnceCallback. So we should find a better solution (e.g. gmock supporting move-only types [1]) before we do massive migration.

Also, media::RunCallback [2] doesn't work with OnceCallback either. That needs to be fixed as well.

[1] https://github.com/google/googletest/issues/395
[2] https://cs.chromium.org/chromium/src/media/base/gmock_callback_support.h

Comment 1 by xhw...@chromium.org, Aug 28 2017

Blockedon: 759710

Comment 2 by xhw...@chromium.org, Jun 13 2018 

Note that issue 759710 is already fixed so gmock supports move-only types. Also media::RunCallback has OnceCallback support with media::RunOnceCallback.

Therefore there's no blocking issue for media to use base::OnceCallback now.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 15 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/06e4121112d87486828cd5811a7700902fb984ec

commit 06e4121112d87486828cd5811a7700902fb984ec
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Fri Jun 15 01:44:55 2018

MSE: Modernize base::Bind usage for StreamParser callbacks

Since upcoming changeType() work will refactor some of the current code
that creates and passes callbacks to StreamParser::Init(), this CL
modernizes related base::Bind and base::Callback usage to either
base::Bind{Once,Repeating} and base::{Once,Repeating}Callback.

Note that the encrypted media init data callback is the same underlying
type in StreamParser as in Demuxer, so this CL also modernizes the
binding for the Demuxer version of that type.

base::Bind count in //media: before=1425, after=1384

BUG=714018,751838, 605134 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ie8c3ef44a836c5bfa3f34ff300da760dd7f552c1
Reviewed-on: https://chromium-review.googlesource.com/1099935
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567507}
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/base/mock_filters.h
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/base/stream_parser.h
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/filters/chunk_demuxer.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/filters/chunk_demuxer_unittest.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/filters/demuxer_perftest.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/filters/ffmpeg_demuxer_unittest.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/filters/source_buffer_state.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/filters/source_buffer_state.h
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/filters/source_buffer_state_unittest.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/formats/common/stream_parser_test_base.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/formats/mp2t/mp2t_stream_parser.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/formats/mp2t/mp2t_stream_parser.h
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/formats/mp2t/mp2t_stream_parser_unittest.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/formats/mp4/mp4_stream_parser.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/formats/mp4/mp4_stream_parser.h
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/formats/mp4/mp4_stream_parser_unittest.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/formats/mpeg/mpeg_audio_stream_parser_base.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/formats/mpeg/mpeg_audio_stream_parser_base.h
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/formats/webm/webm_stream_parser.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/formats/webm/webm_stream_parser.h
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/formats/webm/webm_stream_parser_unittest.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/test/mock_media_source.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/test/pipeline_integration_fuzzertest.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/test/pipeline_integration_test.cc
[modify] https://crrev.com/06e4121112d87486828cd5811a7700902fb984ec/media/test/pipeline_integration_test_base.cc
►

Sign in to add a comment