New issue
Advanced search Search tips

Issue 748751 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

MSE audio config change of samplerate produces spammy "Large timestamp gap detected"

Project Member Reported by wolenetz@chromium.org, Jul 25 2017

Issue description

I detected this while testing a local CL enabling FLAC-in-ISOBMFF for MSE.

To make sure this wasn't due to something in my FLAC CL, I repro'ed the same issue using MSE with AAC-in-MP4 (fragmented) on Chrome (Linux) 59.0.3071.115 (Official Build) (64-bit).

Repro steps:
1) extract attached large_timestamp_gap_spam.tar.gz
2) python -m SimpleHttpServer 8000
3) browse to http://localhost:8000/test.html
4) Observe spam in chrome://media-internals at the config change point, but otherwise the audio plays twice just fine.

See comments in the test.html regarding details.
Basics of the test structure:
Sequence appendmode append of:
InitSegment+MediaSegment(s) of AAC-LC 22050Hz media, then
InitSegment+MediaSegment(s) of AAC-LC 44100Hz media.
Play.

Example spam:
00:00:02 800	error	Large timestamp gap detected; may cause AV sync to drift. time:2949453us expected:3018594us delta:-69141us
00:00:02 845	error	Large timestamp gap detected; may cause AV sync to drift. time:2971812us expected:3065034us delta:-93222us
00:00:02 860	error	Large timestamp gap detected; may cause AV sync to drift. time:2995961us expected:3111473us delta:-115512us
...
00:00:05 425	error	Large timestamp gap detected; may cause AV sync to drift. time:5573059us expected:8266303us delta:-2693244us
00:00:05 446	error	Large timestamp gap detected; may cause AV sync to drift. time:5596075us expected:8312743us delta:-2716668us
00:00:05 485	error	Large timestamp gap detected; may cause AV sync to drift. time:5619295us expected:8359183us delta:-2739888us

It seems to me that the DecoderStreamTraits<DemuxerStream::AUDIO>'s audio_timestamp_validator_ might be missing a necessary reset somewhere around the kConfigChanged processing (local additional logging confirmed that no OnStreamReset() was done after the very first at beginning of playback).
 
large_timestamp_gap_spam.tar.gz
101 KB Download
Some similar test needs to be automated as part of fixing this bug, please.
Labels: M-62
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 25 2017

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

commit 7721535c93622943b37b24ee38c516811ecbdadd
Author: Chris Cunningham <chcunningham@chromium.org>
Date: Fri Aug 25 22:37:50 2017

Reset AudioTimestampValidator upon config change

The validator relies on fields from the AudioDecoderConfig to set
timestamp expectations. When the config changes, simply recreate the
validator with the latest config. 

Neglecting to update the config can lead to incorrect detection of
timestamp gaps, which leads to log spam.

TEST=new unit test
BUG= 748751 

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Id2de8d1df4307fda95d924e06ae975a715d7cfe9
Reviewed-on: https://chromium-review.googlesource.com/634509
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497569}
[modify] https://crrev.com/7721535c93622943b37b24ee38c516811ecbdadd/media/filters/decoder_stream.cc
[modify] https://crrev.com/7721535c93622943b37b24ee38c516811ecbdadd/media/filters/decoder_stream_traits.cc
[modify] https://crrev.com/7721535c93622943b37b24ee38c516811ecbdadd/media/filters/decoder_stream_traits.h
[modify] https://crrev.com/7721535c93622943b37b24ee38c516811ecbdadd/media/test/pipeline_integration_test.cc
[modify] https://crrev.com/7721535c93622943b37b24ee38c516811ecbdadd/media/test/pipeline_integration_test_base.h

Status: Fixed (was: Assigned)

Sign in to add a comment