1424416 - Audio in webm file is jerky when buffered time is low

Status: RESOLVED DUPLICATE of bug 1400674

(Core :: Audio/Video: Playback, defect, P3)

Description

[john357smith]

Attachment: index.html

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.97 Safari/537.36 Vivaldi/1.94.1008.34

Steps to reproduce:

1. Use combination of MSE and SourceBuffer.appendBuffer() for creating a webm/VP8 AV stream. 
2. Let javascript code control the logic of pausing and playing the content - don't stop it with pause button in player or .pause() function
3. Data with appendBuffer() will be appended with frequency just to keep AV buffer under 1 second

Code example of .html file and .webm file is attached. When value of C_MIN_PLAY_BUFFER_TIME is increased to above 2.5 second the problem disappear.

This problem is probably related to https://bugzilla.mozilla.org/show_bug.cgi?id=1400674 where only half of the problem was solved (AV syncing).

The problem occurred with Firefox 49.0. Version 48.0.2 works without any problem.



Actual results:

Played sound from .webm file is very jerky.


Expected results:

Played sound is smooth and without any drop-outs.

Comment 1

[john357smith]

Attachment: test.webm

Comment 2

[john357smith]

Attachment: test-av-384k-44100Hz-1ch-320x240-30fps-10kfr.webm

Comment 3

[john357smith]

Summary of what I was able to find regarding this problem:

- the root cause of this problem is discrepancy between default time scale in FF code vs. TimecodeScale in input .webm/.mkv stream. Problem occurs when a length of audio samples is converted to time units in VorbisDecoder.cpp:

  auto duration = FramesToTimeUnit(frames, rate);

  For example when audio rate is 44100, number of frames is 1024 the result of duration is 23219. But because .webm/.mkv file is using TimecodeScale 1000000 (milliseconds precision) the time (pts) of each frame is incremented only by 23 (= 23000 in microseconds). Each audio frame time is thus 219 usecs behind the real audio length. 

- when decoder.mDecoder->Drain() in  MediaFormatReader.cpp doesn't provide any data it tries to seek to the last sample time ("LOG("Seeking to last sample time: %" PRId64, decoder.mLastDecodedSampleTime.ref().mStart.ToMicroseconds());"). But because the last received sample in output overlapps the last processed sample the result is dropped audio sample ("LOGV("Internal Seeking: Dropping %s frame time:%f wanted:%f (kf:%d)",") which causing that jerking in audio output. 

- to solve this problem you can't increase the TimecodeScale in .webm/.mkv file because with milliseconds precision the cluster can be only 32 seconds long (because of only 16 bits long Timecode in SimpleBlock), with microseconds precision you get only 32 milliseconds long cluster which is far too short.

Could somebody dive into the code and find a best solution for this?

Comment 4

[john357smith]

It has been almost a month and I haven't got any feedback on this problem yet - could somebody look at it? Thanks.

Comment 5

[Andreas Pehrson [:pehrsons]]

jya, is this something you can triage and/or look at?

Comment 6

[Ralph Giles (:rillian)]

Thanks for your patience. I see the 'Internal Seeking: Dropping Audio frame' with MOZ_LOG=MediaFormatReader:5 with your attached examples in Firefox 57 (release) although I don't hear any glitches.

Vorbis is less popular than Opus now, but presumedly the same issue applies there. We don't want glitches in MSE videos.

Comment 7

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: Bug 1424416 - Add RequiresDrainning method to MediaDataDecoder and update impls.

Add RequiresDraining method, which indicates if a decoder requires draining in
order to output all frames in the event of an EOS or running out of data. By
default decoders indicate they require draining. This provides safe default
behaviour.

For a number of decoders we can safely avoid draining, override the method for
these. This will allow users of these decoders to simplify handling and offer
performance improvements.

Review commit: https://reviewboard.mozilla.org/r/235430/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/235430/

Comment 8

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: Bug 1424416 - MediaFormatReader does not drain decoders that do not require it.

Use the new RequiresDraining() method on MediaDataDecoder to avoid unneeded
drains on decoders in the MediaFormatReader (MFR). This should mitigate issues
with webm/mkv audio where low sites appending small amounts of data at a time
can incur a lot of MFR internal seeks and frame drops when running out of data.

Review commit: https://reviewboard.mozilla.org/r/235432/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/235432/

Comment 9

[Jean-Yves Avenard [:jya]]

Comment on attachment 8966734 [details]
Bug 1424416 - Add RequiresDrainning method to MediaDataDecoder and update impls.

https://reviewboard.mozilla.org/r/235430/#review241128

the MediaFormatReader already tracks if a MDD requires draining or not. and only calls drain when needed.
https://searchfox.org/mozilla-central/source/dom/media/MediaFormatReader.cpp#2551

in any case, it should be up to the caller to know if all input data got an output

Comment 10

[Jean-Yves Avenard [:jya]]

Comment on attachment 8966735 [details]
Bug 1424416 - MediaFormatReader does not drain decoders that do not require it.

https://reviewboard.mozilla.org/r/235432/#review241130

::: dom/media/MediaFormatReader.cpp:2559
(Diff revision 1)
>      LOGV("Draining %s with nothing to drain", TrackTypeToStr(aTrack));
>      decoder.mDrainState = DrainState::DrainAborted;
>      ScheduleUpdate(aTrack);
>      return;
>    }
> +  if (!decoder.mDecoder->RequiresDrainning()) {

if theres nothing to drain, then the number of output equal the number of input, which is a case handled above.

Comment 11

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Comment on attachment 8966735 [details]
Bug 1424416 - MediaFormatReader does not drain decoders that do not require it.

https://reviewboard.mozilla.org/r/235432/#review241130

> if theres nothing to drain, then the number of output equal the number of input, which is a case handled above.

This same approach was attempted here: https://reviewboard.mozilla.org/r/189180/diff/1#index_header, why is this not sensible? This also avoids issues with audio decoders that still require draining. This check is not sufficient to stop us draining various audio decoders such as the ones causing this bug, should we still be draining these decoders?

Comment 12

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Before I look for an alternate approach, john357smith, are you still seeing issues with this in the current release? I see in the original report you mention release 49, is this an issue post 57? As noted above, there is dropping in the logs, but I do not hear any glitches in output.

Comment 13

[john357smith]

Yes, you are right now is problem gone! Problem disappeared in release 58. I tried various audio rates and all works smooth and without any drop-outs. I think we can close this bug.

Thanks.

Comment 14

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Think this was probably fixed by the changes in Bug 1400674. Some of the discussion in this and the other bug indicated follow up work may need to be done, but we're no longer seeing glitches sounds like it's fixed and follow up is not required.

Comment 15

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Clearing the ni since we've resolved.