New issue
Advanced search Search tips

Issue 865926 link

Starred by 1 user
Status: Fixed
Owner:
wdzierza...@opera.com
Closed: Jul 23
Cc:
dalecur...@chromium.org
wolenetz@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Audio timestamp validator reset too soon on config change

Project Member Reported by wdzierza...@opera.com, Jul 20 
In a number of AudioOnly/MSEChangeTypeTest.* tests I'm getting the following assertion failure from AudioTimestampValidator. This seems triggered by the particular – but correct – behavior of the media::AudioDecoder I'm working with. DecoderSteam signals a config change but then flushes the decoder, which is expected to produce more decoded samples. FFmpegAudioDecoder (used in the original test) doesn't produce more samples when flushed, while the other decoder (used in my tests) does. In the latter case, the audio timestamp validator is in an inconsistent state, because it has just been reset by the config change that has already been signaled.

[4660:3192:0717/112420.854:91567640:FATAL:audio_timestamp_validator.cc(136)] Check failed: audio_base_ts_ != kNoTimestamp (-9.22337e+12 s vs. -9.22337e+12 s)
Backtrace:
        base::debug::StackTrace::StackTrace [0x00007FFCAF04F835+101] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\debug\stack_trace_win.cc:286)
        base::debug::StackTrace::StackTrace [0x00007FFCAF04E86F+31] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\debug\stack_trace.cc:199)
        logging::LogMessage::~LogMessage [0x00007FFCAF0BC126+134] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\logging.cc:593)
        media::AudioTimestampValidator::RecordOutputDuration [0x00007FFC93764DAF+255] (C:\Users\wdzierzanowski\src\opera\chromium\src\media\filters\audio_timestamp_validator.cc:139)
        media::DecoderStreamTraits<media::DemuxerStream::AUDIO>::OnDecodeDone [0x00007FFC937C7175+37] (C:\Users\wdzierzanowski\src\opera\chromium\src\media\filters\decoder_stream_traits.cc:90)
        media::DecoderStream<media::DemuxerStream::AUDIO>::OnDecodeOutputReady [0x00007FFC937B74B8+1048] (C:\Users\wdzierzanowski\src\opera\chromium\src\media\filters\decoder_stream.cc:568)
        base::internal::FunctorTraits<void (media::DecoderStream<DemuxerStream::AUDIO>::*)(const scoped_refptr<media::AudioBuffer> &),void>::Invoke<void (media::DecoderStream<DemuxerStream::AUDIO>::*)(const scoped_refptr<media::AudioBuffer> &),const base::WeakPtr [0x00007FFC937C5228+72] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\bind_internal.h:507)
        base::internal::InvokeHelper<1,void>::MakeItSo<void (media::DecoderStream<DemuxerStream::AUDIO>::*const &)(const scoped_refptr<media::AudioBuffer> &),const base::WeakPtr<media::DecoderStream<DemuxerStream::AUDIO> > &,const scoped_refptr<media::AudioBuffer [0x00007FFC937C51C4+100] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\bind_internal.h:630)
        base::internal::Invoker<base::internal::BindState<void (media::DecoderStream<DemuxerStream::AUDIO>::*)(const scoped_refptr<media::AudioBuffer> &),base::WeakPtr<media::DecoderStream<DemuxerStream::AUDIO> > >,void (const scoped_refptr<media::AudioBuffer> &) [0x00007FFC937C5152+98] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\bind_internal.h:681)
        base::internal::Invoker<base::internal::BindState<void (media::DecoderStream<DemuxerStream::AUDIO>::*)(const scoped_refptr<media::AudioBuffer> &),base::WeakPtr<media::DecoderStream<DemuxerStream::AUDIO> > >,void (const scoped_refptr<media::AudioBuffer> &) [0x00007FFC937C501E+78] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\bind_internal.h:663)
        base::RepeatingCallback<void (const scoped_refptr<media::AudioBuffer> &)>::Run [0x00007FFC937CC4DB+91] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\callback.h:129)
        base::internal::FunctorTraits<base::RepeatingCallback<void (const scoped_refptr<media::AudioBuffer> &)>,void>::Invoke<const base::RepeatingCallback<void (const scoped_refptr<media::AudioBuffer> &)> &,const scoped_refptr<media::AudioBuffer> &> [0x00007FFC93954DB3+211] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\bind_internal.h:583)
        base::internal::InvokeHelper<0,void>::MakeItSo<const base::RepeatingCallback<void (const scoped_refptr<media::AudioBuffer> &)> &,const scoped_refptr<media::AudioBuffer> &> [0x00007FFC93954C84+52] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\bind_internal.h:607)
        base::internal::Invoker<base::internal::BindState<base::RepeatingCallback<void (const scoped_refptr<media::AudioBuffer> &)>,scoped_refptr<media::AudioBuffer> >,void ()>::RunImpl<const base::RepeatingCallback<void (const scoped_refptr<media::AudioBuffer> & [0x00007FFC93954C49+73] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\bind_internal.h:681)
        base::internal::Invoker<base::internal::BindState<base::RepeatingCallback<void (const scoped_refptr<media::AudioBuffer> &)>,scoped_refptr<media::AudioBuffer> >,void ()>::Run [0x00007FFC93954B7C+60] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\bind_internal.h:663)
        base::OnceCallback<void ()>::Run [0x00007FFCAEFEC071+97] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\callback.h:100)
        base::debug::TaskAnnotator::RunTask [0x00007FFCAF053C43+915] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\debug\task_annotator.cc:103)
        base::internal::IncomingTaskQueue::RunTask [0x00007FFCAF0F1C44+212] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\message_loop\incoming_task_queue.cc:129)
        base::MessageLoop::RunTask [0x00007FFCAF0FC2C2+946] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\message_loop\message_loop.cc:330)
        base::MessageLoop::DeferOrRunPendingTask [0x00007FFCAF0FCAE3+83] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\message_loop\message_loop.cc:342)
        base::MessageLoop::DoWork [0x00007FFCAF0FCDE4+532] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\message_loop\message_loop.cc:383)
        base::MessagePumpDefault::Run [0x00007FFCAF107D17+119] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\message_loop\message_pump_default.cc:37)
        base::MessageLoop::Run [0x00007FFCAF0FBB25+629] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\message_loop\message_loop.cc:283)
        base::RunLoop::Run [0x00007FFCAF1ED34A+506] (C:\Users\wdzierzanowski\src\opera\chromium\src\base\run_loop.cc:120)
        media::PipelineIntegrationTestBase::RunUntilQuitOrError [0x00007FF7E69DB40B+107] (C:\Users\wdzierzanowski\src\opera\chromium\src\media\test\pipeline_integration_test_base.cc:771)
        media::PipelineIntegrationTestBase::RunUntilQuitOrEndedOrError [0x00007FF7E69D87E6+406] (C:\Users\wdzierzanowski\src\opera\chromium\src\media\test\pipeline_integration_test_base.cc:783)
        media::PipelineIntegrationTestBase::WaitUntilEndedOrError [0x00007FF7E69D85F8+104] (C:\Users\wdzierzanowski\src\opera\chromium\src\media\test\pipeline_integration_test_base.cc:261)
        media::PipelineIntegrationTestBase::WaitUntilOnEnded [0x00007FF7E69D832B+299] (C:\Users\wdzierzanowski\src\opera\chromium\src\media\test\pipeline_integration_test_base.cc:251)
        media::MSEChangeTypeTest::PlayBackToBack [0x00007FF7E5E036B0+3008] (C:\Users\wdzierzanowski\src\opera\chromium\src\media\test\pipeline_integration_test.cc:738)
        media::MSEChangeTypeTest_LegacyByDts_PlayBackToBack_Test::TestBody [0x00007FF7E5E02ACD+93] (C:\Users\wdzierzanowski\src\opera\chromium\src\media\test\pipeline_integration_test.cc:749)

Comment 1 by wdzierza...@opera.com, Jul 20

Status: Started (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 23 

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

commit e318f812b99e7e795be931df6404d35957635aa9
Author: Wojciech Dzierżanowski <wdzierzanowski@opera.com>
Date: Mon Jul 23 08:39:16 2018

Reset timestamp validator when config change is complete

When DecoderStream<DemuxerStream::AUDIO> encounters a config change it needs to
do two things, among others: flush the decoder and reset the audio timestamp
validator. It's important that it does the latter only after processing any
decoder output produced during the flushing.

This CL moves the responsibility of resetting the validator from
DecoderStream to DecoderStreamTraits<DemuxerStream::AUDIO>. It also adds
a test where the decoder's behavior of releasing internally buffered
samples upon flushing is mocked.

Bug:  865926 
Test: New test AudioBufferStreamTest.FlushOnConfigChange doesn't fail assertions. No regressions in media_unittests.

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: I8cbb01df3792ca9de5db8ab637285d1e64188b39
Reviewed-on: https://chromium-review.googlesource.com/1145188
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Wojciech Dzierżanowski <wdzierzanowski@opera.com>
Cr-Commit-Position: refs/heads/master@{#577123}
[modify] https://crrev.com/e318f812b99e7e795be931df6404d35957635aa9/media/filters/BUILD.gn
[add] https://crrev.com/e318f812b99e7e795be931df6404d35957635aa9/media/filters/audio_buffer_stream_unittest.cc
[modify] https://crrev.com/e318f812b99e7e795be931df6404d35957635aa9/media/filters/decoder_stream.cc
[modify] https://crrev.com/e318f812b99e7e795be931df6404d35957635aa9/media/filters/decoder_stream_traits.cc
[modify] https://crrev.com/e318f812b99e7e795be931df6404d35957635aa9/media/filters/decoder_stream_traits.h

Comment 4 by wdzierza...@opera.com, Jul 23

Status: Fixed (was: Started)
►

Sign in to add a comment