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

Issue 16274005: Separate DemuxerStream and VideoDecoder. (Closed)

Created:
7 years, 6 months ago by xhwang
Modified:
7 years, 5 months ago
CC:
chromium-reviews, feature-media-reviews_chromium.org
Visibility:
Public.

Description

Separate DemuxerStream and VideoDecoder. - Change VideoDecoder::Read() to VideoDecoder::Decode. - VideoFrameStream handles DemuxerStream::Read() and VideoDecoder::Decode. BUG=141788 TEST=none Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=210741

Patch Set 1 #

Patch Set 2 : VideoFrameStream ready for review. #

Total comments: 2

Patch Set 3 : rebase; not ready for review #

Patch Set 4 : update GVD #

Patch Set 5 : Move FakeDecoderBuffer to test_helpers.* #

Patch Set 6 : rebase #

Patch Set 7 : Drop MEDIA_EXPORT in Fake* since they are a part of the test. #

Patch Set 8 : rebase only #

Patch Set 9 : GVD: return drained frames before starting over #

Total comments: 6

Patch Set 10 : rebase only #

Patch Set 11 : resolve comments #

Patch Set 12 : fix win64 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+694 lines, -1010 lines) Patch
M media/base/mock_filters.h View 1 2 2 chunks +4 lines, -3 lines 0 comments Download
M media/base/test_helpers.h View 1 2 3 4 2 chunks +13 lines, -0 lines 0 comments Download
M media/base/test_helpers.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +38 lines, -0 lines 0 comments Download
M media/base/video_decoder.h View 1 2 3 4 5 6 7 2 chunks +19 lines, -19 lines 0 comments Download
M media/filters/decrypting_demuxer_stream.h View 1 2 2 chunks +4 lines, -6 lines 0 comments Download
M media/filters/decrypting_video_decoder.h View 5 chunks +6 lines, -13 lines 0 comments Download
M media/filters/decrypting_video_decoder.cc View 1 2 10 chunks +16 lines, -59 lines 0 comments Download
M media/filters/decrypting_video_decoder_unittest.cc View 1 2 13 chunks +29 lines, -103 lines 0 comments Download
M media/filters/fake_demuxer_stream.h View 1 2 3 4 5 6 2 chunks +1 line, -2 lines 0 comments Download
M media/filters/fake_demuxer_stream.cc View 1 2 3 4 3 chunks +3 lines, -13 lines 0 comments Download
M media/filters/fake_video_decoder.h View 1 2 3 4 5 6 4 chunks +4 lines, -14 lines 0 comments Download
M media/filters/fake_video_decoder.cc View 1 2 3 4 6 chunks +28 lines, -84 lines 0 comments Download
M media/filters/fake_video_decoder_unittest.cc View 1 2 3 4 10 chunks +101 lines, -127 lines 0 comments Download
M media/filters/ffmpeg_video_decoder.h View 4 chunks +7 lines, -12 lines 0 comments Download
M media/filters/ffmpeg_video_decoder.cc View 1 2 11 chunks +14 lines, -66 lines 0 comments Download
M media/filters/ffmpeg_video_decoder_unittest.cc View 1 2 11 chunks +104 lines, -166 lines 0 comments Download
M media/filters/gpu_video_decoder.h View 1 2 3 4 5 6 7 5 chunks +5 lines, -13 lines 0 comments Download
M media/filters/gpu_video_decoder.cc View 1 2 3 4 5 6 7 8 21 chunks +50 lines, -91 lines 0 comments Download
M media/filters/video_decoder_selector.cc View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M media/filters/video_frame_stream.h View 1 2 3 4 5 6 7 8 9 10 5 chunks +23 lines, -14 lines 0 comments Download
M media/filters/video_frame_stream.cc View 1 2 3 4 5 6 7 10 chunks +142 lines, -97 lines 0 comments Download
M media/filters/video_frame_stream_unittest.cc View 1 2 6 chunks +23 lines, -5 lines 0 comments Download
M media/filters/video_renderer_base_unittest.cc View 1 2 3 4 5 6 7 6 chunks +11 lines, -4 lines 0 comments Download
M media/filters/vpx_video_decoder.h View 1 2 4 chunks +8 lines, -13 lines 0 comments Download
M media/filters/vpx_video_decoder.cc View 1 2 10 chunks +34 lines, -79 lines 0 comments Download
M media/media.gyp View 1 2 3 4 5 4 chunks +5 lines, -5 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
xhwang
Disclaimer: Work in progress. Don't review all files :) This is the last CL of ...
7 years, 6 months ago (2013-06-08 00:45:23 UTC) #1
scherkus (not reviewing)
nice -- it looks like we could simplify FFVD etc.. in follow up CLs re: ...
7 years, 6 months ago (2013-06-12 01:06:10 UTC) #2
xhwang
This CL is ready for full review. GVD works well but needs the fix in ...
7 years, 5 months ago (2013-07-03 08:01:34 UTC) #3
scherkus (not reviewing)
lgtm w/ nits nice stuff! https://codereview.chromium.org/16274005/diff/98001/media/base/decoder_buffer.cc File media/base/decoder_buffer.cc (right): https://codereview.chromium.org/16274005/diff/98001/media/base/decoder_buffer.cc#newcode9 media/base/decoder_buffer.cc:9: #include "media/base/video_decoder_config.h" remove? https://codereview.chromium.org/16274005/diff/98001/media/base/decoder_buffer.h ...
7 years, 5 months ago (2013-07-09 01:18:39 UTC) #4
xhwang
https://codereview.chromium.org/16274005/diff/98001/media/base/decoder_buffer.cc File media/base/decoder_buffer.cc (right): https://codereview.chromium.org/16274005/diff/98001/media/base/decoder_buffer.cc#newcode9 media/base/decoder_buffer.cc:9: #include "media/base/video_decoder_config.h" On 2013/07/09 01:18:39, scherkus wrote: > remove? ...
7 years, 5 months ago (2013-07-09 20:00:04 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xhwang@chromium.org/16274005/112001
7 years, 5 months ago (2013-07-09 20:09:53 UTC) #6
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 5 months ago (2013-07-09 20:14:04 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xhwang@chromium.org/16274005/130001
7 years, 5 months ago (2013-07-09 21:52:25 UTC) #8
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 5 months ago (2013-07-09 21:57:18 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xhwang@chromium.org/16274005/130001
7 years, 5 months ago (2013-07-10 01:10:50 UTC) #10
commit-bot: I haz the power
Change committed as 210741
7 years, 5 months ago (2013-07-10 04:42:48 UTC) #11
kinaba
7 years, 5 months ago (2013-07-10 07:03:18 UTC) #12
Message was sent while issue was closed.
On 2013/07/10 04:42:48, I haz the power (commit-bot) wrote:
> Change committed as 210741

Sorry but reverted this CL. Video decoder test started failing 3 out of 4 times:
http://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%28dbg%29%281%...

[----------] 2 tests from VideoRendererBaseTest
[ RUN      ] VideoRendererBaseTest.DecodeError_Playing
[17632:17636:0709/234011:1648453:FATAL:video_frame_stream.cc(376)] Check failed:
read_cb_.is_null(). 
Backtrace:
	base::win::ShouldCrashOnProcessDetach [0x00F10DD1+2291507]
	base::win::ShouldCrashOnProcessDetach [0x00D5F42E+515472]
	std::list<media::VideoCaptureDevice::Name,std::allocator<media::VideoCaptureDevice::Name>
>::operator= [0x1016244D+1387814]
	std::list<media::VideoCaptureDevice::Name,std::allocator<media::VideoCaptureDevice::Name>
>::operator= [0x10165F6B+1402948]
	std::list<media::VideoCaptureDevice::Name,std::allocator<media::VideoCaptureDevice::Name>
>::operator= [0x10165C42+1402139]
	std::list<media::VideoCaptureDevice::Name,std::allocator<media::VideoCaptureDevice::Name>
>::operator= [0x10165805+1401054]
	base::win::ShouldCrashOnProcessDetach [0x00D3D59F+376577]
	base::win::ShouldCrashOnProcessDetach [0x00DCEE13+972661]
	base::win::ShouldCrashOnProcessDetach [0x00DCF304+973926]
	base::win::ShouldCrashOnProcessDetach [0x00DD0296+977912]
	base::win::ShouldCrashOnProcessDetach [0x00DCA806+954728]
	base::win::ShouldCrashOnProcessDetach [0x00DCE959+971451]
	base::win::ShouldCrashOnProcessDetach [0x00DCE6AE+970768]
	base::win::ShouldCrashOnProcessDetach [0x00D0A509+167531]
	base::win::ShouldCrashOnProcessDetach [0x00DCD981+967395]
	(No symbol) [0x00954408]
	(No symbol) [0x00954138]
	(No symbol) [0x006AD1C3]
	(No symbol) [0x006ACEEE]
	(No symbol) [0x006AFC55]
	(No symbol) [0x009C97DF]
	(No symbol) [0x009B4DEE]
	(No symbol) [0x009B57BD]
	(No symbol) [0x009B5F8F]
	(No symbol) [0x009BC89D]
	(No symbol) [0x009CA327]
	(No symbol) [0x009BB100]
	(No symbol) [0x00995AA0]
	(No symbol) [0x0059FCA4]
	(No symbol) [0x009E22DF]
	(No symbol) [0x009E210F]
	RegisterWaitForInputIdle [0x7C817077+73]

[17632:17636:0709/234011:1648453:FATAL:video_frame_stream.cc(376)] Check failed:
read_cb_.is_null(). 
Backtrace:
	base::win::ShouldCrashOnProcessDetach [0x00F10DD1+2291507]
	base::win::ShouldCrashOnProcessDetach [0x00D5F42E+515472]
	std::list<media::VideoCaptureDevice::Name,std::allocator<media::VideoCaptureDevice::Name>
>::operator= [0x1016244D+1387814]
	std::list<media::VideoCaptureDevice::Name,std::allocator<media::VideoCaptureDevice::Name>
>::operator= [0x10165F6B+1402948]
	std::list<media::VideoCaptureDevice::Name,std::allocator<media::VideoCaptureDevice::Name>
>::operator= [0x10165C42+1402139]
	std::list<media::VideoCaptureDevice::Name,std::allocator<media::VideoCaptureDevice::Name>
>::operator= [0x10165805+1401054]
	base::win::ShouldCrashOnProcessDetach [0x00D3D59F+376577]
	base::win::ShouldCrashOnProcessDetach [0x00DCEE13+972661]
	base::win::ShouldCrashOnProcessDetach [0x00DCF304+973926]
	base::win::ShouldCrashOnProcessDetach [0x00DD0296+977912]
	base::win::ShouldCrashOnProcessDetach [0x00DCA806+954728]
	base::win::ShouldCrashOnProcessDetach [0x00DCE959+971451]
	base::win::ShouldCrashOnProcessDetach [0x00DCE6AE+970768]
	base::win::ShouldCrashOnProcessDetach [0x00D0A509+167531]
	base::win::ShouldCrashOnProcessDetach [0x00DCD981+967395]
	(No symbol) [0x00954408]
	(No symbol) [0x00954138]
	(No symbol) [0x006AD1C3]
	(No symbol) [0x006ACEEE]
	(No symbol) [0x006AFC55]
	(No symbol) [0x009C97DF]
	(No symbol) [0x009B4DEE]
	(No symbol) [0x009B57BD]
	(No symbol) [0x009B5F8F]
	(No symbol) [0x009BC89D]
	(No symbol) [0x009CA327]
	(No symbol) [0x009BB100]
	(No symbol) [0x00995AA0]
	(No symbol) [0x0059FCA4]
	(No symbol) [0x009E22DF]
	(No symbol) [0x009E210F]
	RegisterWaitForInputIdle [0x7C817077+73]

Powered by Google App Engine
This is Rietveld 408576698