Allow seeks to zero on streaming sources.

Source/core/html/HTMLMediaElement.cpp

Index: Source/core/html/HTMLMediaElement.cpp
diff --git a/Source/core/html/HTMLMediaElement.cpp b/Source/core/html/HTMLMediaElement.cpp
index 2a8bc6b446321e6740ccabdf0c2f7afa8509fcaa..fd3ad9414b2adb72efddd4f52360c1b04b79c1a0 100644
--- a/Source/core/html/HTMLMediaElement.cpp
+++ b/Source/core/html/HTMLMediaElement.cpp
@@ -3228,6 +3228,17 @@ PassRefPtrWillBeRawPtr<TimeRanges> HTMLMediaElement::seekable() const
         double maxTimeSeekable = webMediaPlayer()->maxTimeSeekable();
         if (maxTimeSeekable)
             return TimeRanges::create(0, maxTimeSeekable);
+        }

[philipj_slow, 2014/09/10 12:51:18]

Doesn't compile ;)

[DaleCurtis, 2014/09/10 17:45:18]

Heh, whoops, must have hit CtrlZ once too many before uploading.

+
+        // For non-streaming media, emulate a seekable range around zero so looping works.

[philipj_slow, 2014/09/10 12:51:18]

I don't think this is fixing the problem at the appropriate point. I assume
we're talking about finite resources that don't support range requests. When
playback has ended, we will know its duration and that could be reported in the
seekable attribute, right?

In Opera's old GStreamer backend, when we reached end-of-stream and didn't have
a duration yet, we reported the current time as the duration, and the seekable
ranges were simply [0,duration].

[acolwell GONE FROM CHROMIUM, 2014/09/10 17:38:43]

But we don't want to advertise that we can actually seek anywhere inside [0,
duration). The idea is that we are signalling that we only can seek to the
beginning.
The FFmpegDemuxer should be doing similar behavior here (
https://code.google.com/p/chromium/codesearch#chromium/src/media/filters/ffmp...).

[philipj_slow, 2014/09/10 18:42:10]

Oh, is a streaming cache used for non-seekable HTTP resources? I was assuming
that they were written to cache like, say, images and that reaching the end
implied that you have it all. If the response has a Content-Length header, then
one should be able to decide up front which kind of cache is appropriate.

Since there can be different strategies, assuming one here seems suboptimal. The
WebMediaPlayer is closer to the relevant facts, could this logic be put there?

I'm also curious what the real-world case that led to the discovery of this bug
was. If it was something like a short looping wav file, then a new network
request for each loop would be bad.

[DaleCurtis, 2014/09/10 18:56:49]

Some are cached, some may not be depending on flags:

https://code.google.com/p/chromium/codesearch#chromium/src/media/blink/cache_...

Note, this doesn't actually impact what the Cache is actually doing, it just
tries to assume why a resource won't be cached. It's not completely accurate and
errs on the side of thinking something will not be cached. I have another active
task to clean this up, but it should give you an idea of what's going on.

With FFmpegDemuxer, even after playback the duration won't be updated.
ChunkDemuxer won't take this path since resources are appended through
JavaScript, thus a seekable range is always defined. I admit this is a
transitive assumption.

See the bug for more details, but just using a python SimpleHTTPServer with a
short clip will show that looping isn't working with non-range requests. It will
currently hang at ended and never resume played or transition the player into
the paused / error state.

+
+        if (duration() != std::numeric_limits<double>::infinity()) {
+            RefPtrWillBeRawPtr<TimeRanges> ranges = TimeRanges::create();
+            double now = webMediaPlayer()->currentTime();
+            ranges->add(0, std::numeric_limits<double>::epsilon());
+            ranges->add(now, now + std::numeric_limits<double>::epsilon());

[acolwell GONE FROM CHROMIUM, 2014/09/10 17:38:43]

Have you tested this with https://codereview.chromium.org/456343002/ applied? I
would be very surprised if it actually worked the way you want. I think we
should leave this out, just fix the looping problem, and the followup with this
in a different CL once https://codereview.chromium.org/456343002/ lands.

[DaleCurtis, 2014/09/10 17:45:18]

I haven't, but that CL just removes the noSeekRequired flag which this code
transitively bypasses already. I'll patch it in and give it a shot to be sure
though.

How are you proposing to fix the issue if we leave this out? It's the actual fix
:)

[acolwell GONE FROM CHROMIUM, 2014/09/10 18:33:19]

You only need the [0,epsilon) range for loop to work right? I thought the [now,
now + epsilon) is just a convenience if the webapp tries to seek elsewhere.

[acolwell GONE FROM CHROMIUM, 2014/09/10 18:41:05]

Oh right. I didn't happen to notice that the nearest() call was after this code
and assumed the "seek to now" caused by nearest would end up on that path.
Sorry.

It doesn't look like Pipeline special cases "seek to now" so won't this end up
causing the FFmpegDemuxer to do a completely new fetch for the resource and
potentially download the whole thing again?

[DaleCurtis, 2014/09/10 18:56:49]

Ah I thought you were asking to remove all ranges, yes, moving the now,now range
to another patch or finding a different solution is fine me.

Technically yes, but since this is such a short resource it's all actually
cached. We're just not allowing seeks into it because we can't assume that
(though we could make some guess based on duration).

Additionally, I'd like to see us special case seek to now before landing the
patch you linked to avoid unnecessary seeks. I think it should land before hand
to avoid issues.

+            return ranges;
+        }
     }
     return TimeRanges::create();
 }