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

Issue 53126: Demuxer stream now uses a callback instead of Assignable template. Cleaning ... (Closed)

Created:
11 years, 9 months ago by ralphl
Modified:
9 years, 7 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Demuxer stream now uses a callback instead of Assignable template. Cleaning up Pipeine interfaces to conform with Chrome Base design. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=12652

Patch Set 1 #

Total comments: 4

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+74 lines, -57 lines) Patch
M media/base/filters.h View 1 2 chunks +4 lines, -2 lines 0 comments Download
M media/base/mock_media_filters.h View 1 1 chunk +1 line, -1 line 0 comments Download
M media/filters/decoder_base.h View 1 2 chunks +2 lines, -3 lines 0 comments Download
M media/filters/ffmpeg_demuxer.h View 1 2 chunks +5 lines, -5 lines 0 comments Download
M media/filters/ffmpeg_demuxer.cc View 1 5 chunks +19 lines, -17 lines 0 comments Download
M media/filters/ffmpeg_demuxer_unittest.cc View 1 3 chunks +43 lines, -29 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
ralphl
11 years, 9 months ago (2009-03-27 00:25:58 UTC) #1
scherkus (not reviewing)
LGTM with some comments http://codereview.chromium.org/53126/diff/1/6 File media/base/filters.h (right): http://codereview.chromium.org/53126/diff/1/6#newcode162 Line 162: virtual void Read(Callback1<Buffer*>::Type* read_callback) ...
11 years, 9 months ago (2009-03-27 01:46:07 UTC) #2
ralphl
11 years, 9 months ago (2009-03-27 05:36:36 UTC) #3
Changed to scoped_ptr in demuxer inner loop.  Since the tree is closed, I
decided to upload one more time to make 100% sure the try servers were happy
with the code.  

I'll check it in as soon as the tree opens.

http://codereview.chromium.org/53126/diff/1/6
File media/base/filters.h (right):

http://codereview.chromium.org/53126/diff/1/6#newcode162
Line 162: virtual void Read(Callback1<Buffer*>::Type* read_callback) = 0;
On 2009/03/27 01:46:08, scherkus wrote:
> here's just a crazy thought.. what if we used scoped_refptr<Buffer> instead? 
> I'm not sure if its worth the hassle.

In cases where the caller passes a pointer, the caller is always guaranteeing
that the pointer is valid for the duration of the call.  In the case of ref
counted objects, the called procedure must reference the object within the call
to claim ownership.

http://codereview.chromium.org/53126/diff/1/3
File media/filters/ffmpeg_demuxer.cc (right):

http://codereview.chromium.org/53126/diff/1/3#newcode148
Line 148: Callback1<Buffer*>::Type* read_callback;
On 2009/03/27 01:46:08, scherkus wrote:
> may consider using scoped_ptr for this

Good idea.  Changed it.

Powered by Google App Engine
This is Rietveld 408576698