From 058333565d3f0e25e3a4d96789dd76c3c58a1c31 Mon Sep 17 00:00:00 2001 From: Oliver Woodman Date: Tue, 15 Jul 2014 13:55:59 +0100 Subject: [PATCH] Use bits/sec instead of bytes/sec for format bandwidth. Why: This was a bad initial choice. Manifests typically define bandwidth in bits/sec. If you divide by 8 then you're throwing away information due to rounding. Unfortunately it turns out that SegmentTemplate based manifests require you to be able to recall the bitrate exactly (because it's substituted in during segment URL construction). Medium term: We should consider converting all our bandwidth estimation over to bits/sec as well. Note1: Also changed Period id to be a string, to match the mpd spec. Note2: Made small optimization in FormatEvaluator to not consider discarding the first chunk (durationBeforeThisSegmentUs will always be negative, and even in the error case where it's not, removing the first thunk should be an error). --- .../android/exoplayer/chunk/Format.java | 36 ++++++++----------- .../exoplayer/chunk/FormatEvaluator.java | 10 +++--- .../MediaPresentationDescriptionParser.java | 4 +-- .../android/exoplayer/dash/mpd/Period.java | 8 ++--- .../SmoothStreamingChunkSource.java | 6 ++-- 5 files changed, 28 insertions(+), 36 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer/chunk/Format.java b/library/src/main/java/com/google/android/exoplayer/chunk/Format.java index b33fb3aeea..ea8344c6c2 100644 --- a/library/src/main/java/com/google/android/exoplayer/chunk/Format.java +++ b/library/src/main/java/com/google/android/exoplayer/chunk/Format.java @@ -31,7 +31,7 @@ public class Format { @Override public int compare(Format a, Format b) { - return b.bandwidth - a.bandwidth; + return b.bitrate - a.bitrate; } } @@ -67,45 +67,37 @@ public class Format { public final int audioSamplingRate; /** - * The average bandwidth in bytes per second. + * The average bandwidth in bits per second. */ + public final int bitrate; + + /** + * The average bandwidth in bytes per second. + * + * @deprecated Use {@link #bitrate}. However note that the units of measurement are different. + */ + @Deprecated public final int bandwidth; /** - * @deprecated Format identifiers are now strings. - * * @param id The format identifier. * @param mimeType The format mime type. * @param width The width of the video in pixels, or -1 for non-video formats. * @param height The height of the video in pixels, or -1 for non-video formats. * @param numChannels The number of audio channels, or -1 for non-audio formats. * @param audioSamplingRate The audio sampling rate in Hz, or -1 for non-audio formats. - * @param bandwidth The average bandwidth of the format in bytes per second. - */ - @Deprecated - public Format(int id, String mimeType, int width, int height, int numChannels, - int audioSamplingRate, int bandwidth) { - this(String.valueOf(id), mimeType, width, height, numChannels, audioSamplingRate, bandwidth); - } - - /** - * @param id The format identifier. - * @param mimeType The format mime type. - * @param width The width of the video in pixels, or -1 for non-video formats. - * @param height The height of the video in pixels, or -1 for non-video formats. - * @param numChannels The number of audio channels, or -1 for non-audio formats. - * @param audioSamplingRate The audio sampling rate in Hz, or -1 for non-audio formats. - * @param bandwidth The average bandwidth of the format in bytes per second. + * @param bitrate The average bandwidth of the format in bits per second. */ public Format(String id, String mimeType, int width, int height, int numChannels, - int audioSamplingRate, int bandwidth) { + int audioSamplingRate, int bitrate) { this.id = Assertions.checkNotNull(id); this.mimeType = mimeType; this.width = width; this.height = height; this.numChannels = numChannels; this.audioSamplingRate = audioSamplingRate; - this.bandwidth = bandwidth; + this.bitrate = bitrate; + this.bandwidth = bitrate / 8; } /** diff --git a/library/src/main/java/com/google/android/exoplayer/chunk/FormatEvaluator.java b/library/src/main/java/com/google/android/exoplayer/chunk/FormatEvaluator.java index 1b8b9d9082..1a87b9a142 100644 --- a/library/src/main/java/com/google/android/exoplayer/chunk/FormatEvaluator.java +++ b/library/src/main/java/com/google/android/exoplayer/chunk/FormatEvaluator.java @@ -236,8 +236,8 @@ public interface FormatEvaluator { : queue.get(queue.size() - 1).endTimeUs - playbackPositionUs; Format current = evaluation.format; Format ideal = determineIdealFormat(formats, bandwidthMeter.getEstimate()); - boolean isHigher = ideal != null && current != null && ideal.bandwidth > current.bandwidth; - boolean isLower = ideal != null && current != null && ideal.bandwidth < current.bandwidth; + boolean isHigher = ideal != null && current != null && ideal.bitrate > current.bitrate; + boolean isLower = ideal != null && current != null && ideal.bitrate < current.bitrate; if (isHigher) { if (bufferedDurationUs < minDurationForQualityIncreaseUs) { // The ideal format is a higher quality, but we have insufficient buffer to @@ -247,11 +247,11 @@ public interface FormatEvaluator { // We're switching from an SD stream to a stream of higher resolution. Consider // discarding already buffered media chunks. Specifically, discard media chunks starting // from the first one that is of lower bandwidth, lower resolution and that is not HD. - for (int i = 0; i < queue.size(); i++) { + for (int i = 1; i < queue.size(); i++) { MediaChunk thisChunk = queue.get(i); long durationBeforeThisSegmentUs = thisChunk.startTimeUs - playbackPositionUs; if (durationBeforeThisSegmentUs >= minDurationToRetainAfterDiscardUs - && thisChunk.format.bandwidth < ideal.bandwidth + && thisChunk.format.bitrate < ideal.bitrate && thisChunk.format.height < ideal.height && thisChunk.format.height < 720 && thisChunk.format.width < 1280) { @@ -280,7 +280,7 @@ public interface FormatEvaluator { long effectiveBandwidth = computeEffectiveBandwidthEstimate(bandwidthEstimate); for (int i = 0; i < formats.length; i++) { Format format = formats[i]; - if (format.bandwidth <= effectiveBandwidth) { + if ((format.bitrate / 8) <= effectiveBandwidth) { return format; } } diff --git a/library/src/main/java/com/google/android/exoplayer/dash/mpd/MediaPresentationDescriptionParser.java b/library/src/main/java/com/google/android/exoplayer/dash/mpd/MediaPresentationDescriptionParser.java index 9f8e1e2c0c..3d216d4839 100644 --- a/library/src/main/java/com/google/android/exoplayer/dash/mpd/MediaPresentationDescriptionParser.java +++ b/library/src/main/java/com/google/android/exoplayer/dash/mpd/MediaPresentationDescriptionParser.java @@ -111,7 +111,7 @@ public class MediaPresentationDescriptionParser extends DefaultHandler { private Period parsePeriod(XmlPullParser xpp, String contentId, Uri parentBaseUrl, long mediaPresentationDuration) throws XmlPullParserException, IOException { Uri baseUrl = parentBaseUrl; - int id = parseInt(xpp, "id"); + String id = xpp.getAttributeValue(null, "id"); long start = parseDurationMs(xpp, "start", 0); long duration = parseDurationMs(xpp, "duration", mediaPresentationDuration); @@ -214,7 +214,7 @@ public class MediaPresentationDescriptionParser extends DefaultHandler { List segmentTimelineList) throws XmlPullParserException, IOException { Uri baseUrl = parentBaseUrl; String id = xpp.getAttributeValue(null, "id"); - int bandwidth = parseInt(xpp, "bandwidth") / 8; + int bandwidth = parseInt(xpp, "bandwidth"); int audioSamplingRate = parseInt(xpp, "audioSamplingRate"); int width = parseInt(xpp, "width"); int height = parseInt(xpp, "height"); diff --git a/library/src/main/java/com/google/android/exoplayer/dash/mpd/Period.java b/library/src/main/java/com/google/android/exoplayer/dash/mpd/Period.java index 4b33161acb..71294204e7 100644 --- a/library/src/main/java/com/google/android/exoplayer/dash/mpd/Period.java +++ b/library/src/main/java/com/google/android/exoplayer/dash/mpd/Period.java @@ -23,7 +23,7 @@ import java.util.List; */ public final class Period { - public final int id; + public final String id; public final long start; @@ -39,16 +39,16 @@ public final class Period { public final long presentationTimeOffset; - public Period(int id, long start, long duration, List adaptationSets) { + public Period(String id, long start, long duration, List adaptationSets) { this(id, start, duration, adaptationSets, null, 0, 0, 0); } - public Period(int id, long start, long duration, List adaptationSets, + public Period(String id, long start, long duration, List adaptationSets, List segmentList, int segmentStartNumber, int segmentTimescale) { this(id, start, duration, adaptationSets, segmentList, segmentStartNumber, segmentTimescale, 0); } - public Period(int id, long start, long duration, List adaptationSets, + public Period(String id, long start, long duration, List adaptationSets, List segmentList, int segmentStartNumber, int segmentTimescale, long presentationTimeOffset) { this.id = id; diff --git a/library/src/main/java/com/google/android/exoplayer/smoothstreaming/SmoothStreamingChunkSource.java b/library/src/main/java/com/google/android/exoplayer/smoothstreaming/SmoothStreamingChunkSource.java index e8ed35d239..d6a70e0364 100644 --- a/library/src/main/java/com/google/android/exoplayer/smoothstreaming/SmoothStreamingChunkSource.java +++ b/library/src/main/java/com/google/android/exoplayer/smoothstreaming/SmoothStreamingChunkSource.java @@ -103,7 +103,7 @@ public class SmoothStreamingChunkSource implements ChunkSource { TrackElement trackElement = streamElement.tracks[trackIndex]; formats[i] = new SmoothStreamingFormat(String.valueOf(trackIndex), trackElement.mimeType, trackElement.maxWidth, trackElement.maxHeight, trackElement.numChannels, - trackElement.sampleRate, trackElement.bitrate / 8, trackIndex); + trackElement.sampleRate, trackElement.bitrate, trackIndex); maxWidth = Math.max(maxWidth, trackElement.maxWidth); maxHeight = Math.max(maxHeight, trackElement.maxHeight); @@ -266,8 +266,8 @@ public class SmoothStreamingChunkSource implements ChunkSource { public final int trackIndex; public SmoothStreamingFormat(String id, String mimeType, int width, int height, - int numChannels, int audioSamplingRate, int bandwidth, int trackIndex) { - super(id, mimeType, width, height, numChannels, audioSamplingRate, bandwidth); + int numChannels, int audioSamplingRate, int bitrate, int trackIndex) { + super(id, mimeType, width, height, numChannels, audioSamplingRate, bitrate); this.trackIndex = trackIndex; }