New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 0 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Oct 2011
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 96861

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
link

Issue 98975: Stop() during Seek() triggers DCHECKs for invalid state transitions

Reported by scherkus@chromium.org, Oct 4 2011 Project Member

Issue description

What steps will reproduce the problem?
1. Run debug build of Chrome
2. Load up a video with http://ascherkus.github.com/iheartvideo/benchmark.html
3. Keep hitting Run button

Eventually I hit the following DCHECK:
[10678:11300:3061991052922:FATAL:composite_filter.cc(278)] Check failed: status_cb_.is_null() ^ callback_.is_null(). 

Not sure how serious this is.
 

Comment 1 by acolwell@chromium.org, Oct 4 2011

This is likely caused by Stop() being called while a Seek() is pending. In general the Filter::Stop() semantics have been that Stop() can be called at any time even if there is an async operation pending. The assumption is that when a Filter gets the Stop() call it will just clear its pending callback and initiate the stop. 

I'm not sure if the change to the ^ hotness slightly changed the behavior or if this was always broken and we never hit it before.

Comment 2 by fischman@chromium.org, Oct 4 2011

Owner: ----
Status: Available
Summary: Stop() during Seek() triggers DCHECKs for invalid state transitions
Sync'ing back to r103375 reliably dies in the same repro recipe (not in the equivalent check), so I don't think the callback change is relevant to this bug report.  I think the bug itself is not super-serious, because it only affects error reporting at tear-down. 

The crash @103375:
[22663:22795:4668320026725:FATAL:ffmpeg_video_decoder.cc(288)] Check failed: state_ != kStopped (7 vs. 7)
Backtrace:
        base::debug::StackTrace::StackTrace() [0x7fd828c84b3e]
        logging::LogMessage::~LogMessage() [0x7fd828c02992]
        media::FFmpegVideoDecoder::ProduceVideoFrame() [0x7fd82a6c2533]
        DispatchToMethod<>() [0x7fd82a6c3ba3]
        RunnableMethod<>::Run() [0x7fd82a6c3a56]
        base::subtle::TaskClosureAdapter::Run() [0x7fd828c52471]
        base::internal::Invoker1<>::DoInvoke() [0x7fd828c0c9d2]
        base::Callback<>::Run() [0x7fd828bddfee]
        MessageLoop::RunTask() [0x7fd828c07a74]
        MessageLoop::DeferOrRunPendingTask() [0x7fd828c07bfa]
        MessageLoop::DoWork() [0x7fd828c07dbb]
        base::MessagePumpDefault::Run() [0x7fd828c1176c]
        MessageLoop::RunInternal() [0x7fd828c076b7]
        MessageLoop::RunHandler() [0x7fd828c06f45]
        MessageLoop::Run() [0x7fd828c06f1d]
        base::Thread::Run() [0x7fd828c57759]
        base::Thread::ThreadMain() [0x7fd828c57889]
        base::(anonymous namespace)::ThreadFunc() [0x7fd828c52a75]
        start_thread [0x7fd8237339ca]
        0x7fd8211a470d

Comment 3 by fischman@google.com, Oct 5 2011

Owner: fischman@chromium.org
Status: Started

Comment 4 by acolwell@chromium.org, Oct 6 2011

Blocking: 96861

Comment 5 by bugdroid1@chromium.org, Oct 7 2011

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=104540

------------------------------------------------------------------------
r104540 | fischman@chromium.org | Fri Oct 07 12:12:21 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/base/composite_filter.cc?r1=104540&r2=104539&pathrev=104540
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/ffmpeg_video_decoder.cc?r1=104540&r2=104539&pathrev=104540
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/media/audio_renderer_impl.cc?r1=104540&r2=104539&pathrev=104540
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/ffmpeg_demuxer.cc?r1=104540&r2=104539&pathrev=104540
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/video_renderer_base.cc?r1=104540&r2=104539&pathrev=104540

Fix hangs & crashes in teardown-during-Seek.

Hangs:
- AudioRendererImpl: Don't hold lock_ in the call to Join() b/c OnStop wants to
  acquire the lock before exiting.
- FFmpegDemuxerStream::Stop: signal EOS to waiting read callbacks (instead of
  silently dropping them on the floor) so decoders can exit instead of stalling
  (and causing renderers to stall).

Crashes:
- CompositeFilter::Stop: clear out the Seek callback when Stopping.
- FFmpegVideoDecoder::ProduceVideoFrame: early-return if already stopped (racing
  with webkit repaints makes this necessary).

Also fixed a bunch of mis-indentation where I saw it.

BUG= 98975 , 98948 , 98955 , 96861 
TEST=trybots, manual


Review URL: http://codereview.chromium.org/8184003
------------------------------------------------------------------------

Comment 6 by fischman@google.com, Oct 7 2011

Status: Fixed

Comment 7 by fischman@google.com, Oct 7 2011

This is fixed in trunk.  Decided w/ scherkus@ to not merge into m15 because:
1) It's unclear what the impact is on mainstream use-cases
2) A merge would be tricky (difficult to ensure correct) b/c of interleaved refactorings (Bind & low-latency audio codepaths, specifically).

If this turns out to be a significant problem in the wild we might reconsider this decision.

Comment 8 by bugdroid1@chromium.org, Oct 17 2011

Project Member
Labels: merge-merged-874
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=105930

------------------------------------------------------------------------
r105930 | fischman@chromium.org | Mon Oct 17 14:47:52 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/874/src/media/base/composite_filter.cc?r1=105930&r2=105929&pathrev=105930
 M http://src.chromium.org/viewvc/chrome/branches/874/src/media/filters/ffmpeg_video_decoder.cc?r1=105930&r2=105929&pathrev=105930
 M http://src.chromium.org/viewvc/chrome/branches/874/src/content/renderer/media/audio_renderer_impl.cc?r1=105930&r2=105929&pathrev=105930
 M http://src.chromium.org/viewvc/chrome/branches/874/src/media/filters/ffmpeg_demuxer.cc?r1=105930&r2=105929&pathrev=105930
 M http://src.chromium.org/viewvc/chrome/branches/874/src/media/filters/video_renderer_base.cc?r1=105930&r2=105929&pathrev=105930

Merge 104540 - Fix hangs & crashes in teardown-during-Seek.

Hangs:
- AudioRendererImpl: Don't hold lock_ in the call to Join() b/c OnStop wants to
acquire the lock before exiting.
- FFmpegDemuxerStream::Stop: signal EOS to waiting read callbacks (instead of
silently dropping them on the floor) so decoders can exit instead of stalling
(and causing renderers to stall).

Crashes:
- CompositeFilter::Stop: clear out the Seek callback when Stopping.
- FFmpegVideoDecoder::ProduceVideoFrame: early-return if already stopped (racing
with webkit repaints makes this necessary).

Also fixed a bunch of mis-indentation where I saw it.

BUG= 98975 ,  98948 ,  98955 ,  96861 
TEST=trybots, manual
Review URL: http://codereview.chromium.org/8294027
------------------------------------------------------------------------

Comment 9 by imasaki@chromium.org, Nov 22 2011

Status: Verified
try hitting run button http://ascherkus.github.com/iheartvideo/benchmark.html using Chrome 16.0.912.47 (Mac). I do not notice anything suspicious.

Comment 10 by bugdroid1@chromium.org, Oct 13 2012

Project Member
Blocking: -chromium:96861 chromium:96861
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.

Comment 11 by bugdroid1@chromium.org, Mar 10 2013

Project Member
Labels: -Area-WebKit -Feature-Media -Mstone-16 Cr-Content Cr-Internals-Media M-16

Comment 12 by bugdroid1@chromium.org, Mar 13 2013

Project Member
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue

Comment 13 by bugdroid1@chromium.org, Apr 6 2013

Project Member
Labels: -Cr-Content Cr-Blink

Sign in to add a comment