Remove locking from FFmpegDemuxerStream and associated TODOs from media code.

Remove locking from FFmpegDemuxerStream and associated TODOs from media code.

As of r168350 the message loop will no longer get blocked meaning we can do away with nasty locks. A side benefit is we can finally remove the forced-task-post from various decoders!

BUG=160640
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=168876

[scherkus (not reviewing), 2012-11-19 19:11:16]

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_a...
File media/filters/decrypting_audio_decoder.cc (left):

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_a...
media/filters/decrypting_audio_decoder.cc:243: // In theory, we don't need to
force post the task here, because we do a
xhwang: let me know if you want to keep this (although I see even less of a
reason to keep it around now!)

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_v...
File media/filters/decrypting_video_decoder.cc (left):

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_v...
media/filters/decrypting_video_decoder.cc:244: // In theory, we don't need to
force post the task here, because we do a
xhwang: ditto

[xhwang, 2012-11-20 17:54:34]

lgtm % nits

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_a...
File media/filters/decrypting_audio_decoder.cc (left):

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_a...
media/filters/decrypting_audio_decoder.cc:243: // In theory, we don't need to
force post the task here, because we do a
On 2012/11/19 19:11:16, scherkus wrote:
> xhwang: let me know if you want to keep this (although I see even less of a
> reason to keep it around now!)

Nop, I don't see a reason to keep it now :) Thanks!

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_a...
File media/filters/decrypting_audio_decoder.h (right):

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_a...
media/filters/decrypting_audio_decoder.h:99: // DecryptAndDecodeBuffer().
update this comment.

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_v...
File media/filters/decrypting_video_decoder.h (right):

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_v...
media/filters/decrypting_video_decoder.h:97: // DecryptAndDecodeBuffer().
ditto

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_audio...
File media/filters/ffmpeg_audio_decoder.cc (left):

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_audio...
media/filters/ffmpeg_audio_decoder.cc:374: DCHECK_EQ(status !=
DemuxerStream::kOk, !buffer) << status;
Do you want to keep this check?

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_demux...
File media/filters/ffmpeg_demuxer.cc (right):

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer.cc:129:
scoped_refptr<DecoderBuffer>(DecoderBuffer::CreateEOSBuffer()));
CreateEOSBuffer() returns scoped_refptr<DecoderBuffer> already. Can we remove
the copy construction here?

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer.cc:156:
scoped_refptr<DecoderBuffer>(DecoderBuffer::CreateEOSBuffer()));
ditto

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer.cc:198: void
FFmpegDemuxerStream::SatisfyPendingReads() {
Add DCHECK(message_loop_->BelongsToCurrentThread()); ?

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer.cc:639: void FFmpegDemuxer::NotifyHasPendingRead()
{
This function is only called once in SatisfyPendingReads(). Do we want to have a
separate function for this?

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_video...
File media/filters/ffmpeg_video_decoder.cc (left):

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_video...
media/filters/ffmpeg_video_decoder.cc:261: DCHECK_EQ(status !=
DemuxerStream::kOk, !buffer) << status;
do we still want to keep this check?

[scherkus (not reviewing), 2012-11-20 19:16:07]

thanks!

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_a...
File media/filters/decrypting_audio_decoder.h (right):

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_a...
media/filters/decrypting_audio_decoder.h:99: // DecryptAndDecodeBuffer().
On 2012/11/20 17:54:34, xhwang wrote:
> update this comment.

Done.

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_v...
File media/filters/decrypting_video_decoder.h (right):

https://codereview.chromium.org/11420070/diff/2001/media/filters/decrypting_v...
media/filters/decrypting_video_decoder.h:97: // DecryptAndDecodeBuffer().
On 2012/11/20 17:54:34, xhwang wrote:
> ditto

Done.

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_audio...
File media/filters/ffmpeg_audio_decoder.cc (left):

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_audio...
media/filters/ffmpeg_audio_decoder.cc:374: DCHECK_EQ(status !=
DemuxerStream::kOk, !buffer) << status;
On 2012/11/20 17:54:34, xhwang wrote:
> Do you want to keep this check?

Done.

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_demux...
File media/filters/ffmpeg_demuxer.cc (right):

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer.cc:129:
scoped_refptr<DecoderBuffer>(DecoderBuffer::CreateEOSBuffer()));
On 2012/11/20 17:54:34, xhwang wrote:
> CreateEOSBuffer() returns scoped_refptr<DecoderBuffer> already. Can we remove
> the copy construction here?

Done.

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer.cc:156:
scoped_refptr<DecoderBuffer>(DecoderBuffer::CreateEOSBuffer()));
On 2012/11/20 17:54:34, xhwang wrote:
> ditto

Done.

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer.cc:198: void
FFmpegDemuxerStream::SatisfyPendingReads() {
On 2012/11/20 17:54:34, xhwang wrote:
> Add DCHECK(message_loop_->BelongsToCurrentThread()); ?

Done.

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer.cc:639: void FFmpegDemuxer::NotifyHasPendingRead()
{
On 2012/11/20 17:54:34, xhwang wrote:
> This function is only called once in SatisfyPendingReads(). Do we want to have
a
> separate function for this?

I think it's fine keep around -- it's clearer than the previous PostDemuxTask()
or calling DemuxTask() directly

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_video...
File media/filters/ffmpeg_video_decoder.cc (left):

https://codereview.chromium.org/11420070/diff/2001/media/filters/ffmpeg_video...
media/filters/ffmpeg_video_decoder.cc:261: DCHECK_EQ(status !=
DemuxerStream::kOk, !buffer) << status;
On 2012/11/20 17:54:34, xhwang wrote:
> do we still want to keep this check?

Done.

[commit-bot: I haz the power, 2012-11-20 19:30:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/11420070/15001

[commit-bot: I haz the power, 2012-11-20 20:15:56]

Retried try job too often for step(s) content_browsertests