Closed Bug 1023697 Opened 5 years ago Closed 5 years ago

Use audio ticks as MediaTime units

Categories

(Core :: Audio/Video, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: karlt, Assigned: karlt)

References

Details

Attachments

(15 files)

1.19 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.89 KB, patch
roc
: review+
Details | Diff | Splinter Review
5.84 KB, patch
roc
: review+
Details | Diff | Splinter Review
2.09 KB, patch
roc
: review+
Details | Diff | Splinter Review
876 bytes, patch
roc
: review+
Details | Diff | Splinter Review
4.39 KB, patch
roc
: review+
Details | Diff | Splinter Review
911 bytes, patch
roc
: review+
Details | Diff | Splinter Review
2.74 KB, patch
roc
: review+
Details | Diff | Splinter Review
8.28 KB, patch
roc
: review+
Details | Diff | Splinter Review
6.45 KB, patch
roc
: review+
Details | Diff | Splinter Review
10.34 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.51 KB, patch
roc
: review+
Details | Diff | Splinter Review
5.23 KB, patch
roc
: review+
Details | Diff | Splinter Review
2.53 KB, patch
roc
: review+
Details | Diff | Splinter Review
5.98 KB, patch
roc
: review+
Details | Diff | Splinter Review
MediaStreamGraph uses the same sample rate for all audio tracks, and
processes all MediaStreams in blocks of time based on this audio rate.

However MediaTime doesn't accurately represent the times for each audio tick,
which can lead to bugs such as
https://bugzilla.mozilla.org/show_bug.cgi?id=983023#c1

By changing MediaTime to use audio ticks for units, a whole class of rounding
errors can be avoided.

It is assumed that video frame rates are always much lower than the audio
rate, and so rounding errors from inaccurate representation in MediaTime would
not be detectable.

(Video can have variable frame rate and so a nominal frame rate of 10^-6 is
(already) used to make video ticks correspond to µs.  µs are not precisely
representable in MediaTime whether units are 2^-20 seconds or audio ticks.)
All these Ticks/Time functions will be moved or removed, but this one is easy.
Attachment #8438153 - Flags: review?(roc)
When MediaTime depends on the audio rate of the graph, the stream will know the
graph rate and so can perform the conversion.

Existing methods in StreamBuffer.h and MediaSegment.h will be removed once
clients have been changed to the new methods.
Attachment #8438155 - Flags: review?(roc)
This allows the conversion from StreamTime to be performed while a stream is
available.
Attachment #8438156 - Flags: review?(roc)
The fake graph needs an implementation of the conversion methods.

The real graph will change to use audio ticks for time in a subsequent patch,
but the fake graph doesn't know about MEDIA_TIME_FRAC_BITS, so that change
can be made now in the fake graph.
Attachment #8438164 - Flags: review?(rjesup)
Here we could use a pointer from track to StreamBuffer and from StreamBuffer to Stream or graph, but we'd only use it for the rate, so this is a bit more efficient.

This would all be unnecessary I think if we ran video tracks at the graph rate also but I haven't done that.
Attachment #8438168 - Flags: review?(roc)
This is just a heuristic, so I don't think it is worth the trouble of calculating different values for different rates.
Attachment #8438171 - Flags: review?(roc)
We could get away with 1 as an initial time when units were 1/(1<<20) seconds
but when graph time units are audio samples, an initial value of 1 will mean
that the first audio block produced has 1 frame fewer.

Perhaps we could use 128 if you'd like to keep the distinction from zero.
Attachment #8438174 - Flags: review?(roc)
This is the last of the series.  There are a number of simplifications possible now but I haven't worked out which ones would conflict with changes in bug 848954 or not.
Attachment #8438176 - Flags: review?(roc)
Depends on: 1024901
You need to log in before you can comment on or make changes to this bug.