WebMediaPlayerAndroid - handling unseekable durations

content/renderer/media/android/webmediaplayer_android.cc

Index: content/renderer/media/android/webmediaplayer_android.cc
diff --git a/content/renderer/media/android/webmediaplayer_android.cc b/content/renderer/media/android/webmediaplayer_android.cc
index af0d25b27bd433d216335118bc5fad3ad8d7f163..76b0fc4a06bedd6792d81ee6b5afe962f2cbbb3a 100644
--- a/content/renderer/media/android/webmediaplayer_android.cc
+++ b/content/renderer/media/android/webmediaplayer_android.cc
@@ -386,6 +386,11 @@ void WebMediaPlayerAndroid::seek(double seconds) {
     return;
   }
 
+  if (new_seek_time >=

[acolwell GONE FROM CHROMIUM, 2014/01/13 16:32:07]

This should not be needed if maxTimeSeekable() never returns a value greater
than std::numeric_limits<int32>max().

[amogh.bihani, 2014/01/14 08:03:09]

Done.

+          base::TimeDelta::FromMilliseconds(std::numeric_limits<int32>max())) {
+    return;
+  }
+
   seeking_ = true;
   seek_time_ = new_seek_time;
 
@@ -502,7 +507,6 @@ double WebMediaPlayerAndroid::maxTimeSeekable() const {
   if (ready_state_ < WebMediaPlayer::ReadyStateHaveMetadata)
     return 0.0;
 
-  // TODO(hclam): If this stream is not seekable this should return 0.

[amogh.bihani, 2014/01/13 11:43:14]

This is handled in this patch automatically

   return duration();

[acolwell GONE FROM CHROMIUM, 2014/01/13 16:32:07]

This does not see right to me. I believe this should be
std::min(std::numeric_limits<int32>max(), duration()) instead so that the
seekable range is never too large. This allow Blink to prevent invalid seek
values from ever being passed to seek().

[amogh.bihani, 2014/01/14 08:03:09]

Done.

 }
 
@@ -619,9 +623,14 @@ void WebMediaPlayerAndroid::OnMediaMetadataChanged(
 
   // Update duration, if necessary, prior to ready state updates that may
   // cause duration() query.
-  // TODO(wolenetz): Correctly handle durations that MediaSourcePlayer
-  // considers unseekable, including kInfiniteDuration().

[amogh.bihani, 2014/01/13 11:43:14]

kInfiniteDuration is 2^63, so this is automatically handled

-  // See http://crbug.com/248396
+
+  // MediaSourcePlayer assumes duration more than 2^31 as unseekable
+  // because java player uses 32-bit integer for time at millisecond resolution
+  if (duration >=

[acolwell GONE FROM CHROMIUM, 2014/01/13 16:32:07]

This does not seem right. The clip duration and the seekable range are 2
different concepts. I don't think it is correct to prevent duration updates just
because the seekable range is smaller than the duration. The duration update
should be allowed to proceed and as long as maxTimeSeekable() is returning the
right information, Blink should prevent seeks from outside the seekable range.
If it isn't then we probably need to make a change to the Blink code instead.

[amogh.bihani, 2014/01/14 08:03:09]

Done.

+          base::TimeDelta::FromMilliseconds(std::numeric_limits<int32>max())) {
+    return;
+  }
+
   if (!ignore_metadata_duration_change_ && duration_ != duration) {
     duration_ = duration;
 
@@ -828,11 +837,15 @@ void WebMediaPlayerAndroid::OnDurationChanged(const base::TimeDelta& duration) {
   // Only MSE |player_type_| registers this callback.
   DCHECK_EQ(player_type_, MEDIA_PLAYER_TYPE_MEDIA_SOURCE);
 
+  // MediaSourcePlayer assumes duration more than 2^31 as unseekable
+  // because java player uses 32-bit integer for time at millisecond resolution
+  if (duration >=
+          base::TimeDelta::FromMilliseconds(std::numeric_limits<int32>max())) {
+    return;
+  }
+
   // Cache the new duration value and trust it over any subsequent duration
   // values received in OnMediaMetadataChanged().
-  // TODO(wolenetz): Correctly handle durations that MediaSourcePlayer
-  // considers unseekable, including kInfiniteDuration().
-  // See http://crbug.com/248396
   duration_ = duration;
   ignore_metadata_duration_change_ = true;