getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.31k stars 168 forks source link

Video files (.mp4) cannot be loaded by Safari #1207

Open robinscholz opened 5 years ago

robinscholz commented 5 years ago

Describe the bug Safari needs the server to support byte-range requests. Instead of simply requesting the video, Safari first sends a byte range request:

Accept: */*
Accept-Encoding: identity
Connection: keep-alive
Range: bytes=0-1

and expects the response status 206. Kirby currently responds with 200. Safari then aborts its request instead of sending various others for the complete range of bytes.

To Reproduce Steps to reproduce the behavior:

  1. Install the starterkit.
  2. Change the files/image.yml blueprint tom include videos
    
    accept:
    mime: image/jpeg, image/png, image/svg+xml, video/mp4

3. Upload a small video.mp4 file to a project subpage.
4. Click on the preview icon to request the file directly via its `/media/` URL

**Expected behavior**
Kirby should respond with code `206` for the initial response to a video file request.

**Kirby Version**
3.0.0-beta-6.21

**Console output**
`Failed to load resource: Plug-in handled load`

**Desktop (please complete the following information):**
 - macOS
 - Safari
 - Version 12.0.1 (14606.2.104.1.1)
robinscholz commented 5 years ago

Closing this, since it seems to be a problem with the PHP localhost server, unrelated to Kirby.

PatricB commented 4 years ago

@robinscholz The same problem occurs with us not only with a PHP localhost server. Can you recommend server settings that worked for you?

hdodov commented 1 year ago

@robinscholz no, it's not a problem with the PHP localhost server.

On the very first request for a video (that's not in the media folder), the request is handled in PHP by Kirby which doesn't add the required Range header, as discussed on Stack Overflow. Therefore, the video doesn't play on Safari.

On subsequent requests, the video file now exists in the media folder, so the request is not getting passed to PHP and is getting served by Apache instead, and it sets all the correct headers (and also returns HTTP 206).

You should reopen this. We have this issue on production and we always have to purge the cache the first time a new video is uploaded because it's incorrectly served.

distantnative commented 1 year ago

Ideas where to tackle this:

$handle = fopen($file, 'rb');
fseek($handle, $start);

if ($end === $size) {
    fpassthru($handle);
} else {
    $step      = 8 * KB_IN_BYTES;
    $remaining = $end - $start;

    while (
        $remaining &&
        !feof($handle) &&
        $chunk = fread($handle, min($step, $remaining))
    ) {
        $remaining -= strlen($step);
        echo $chunk;
        flush();
    }
}

fclose( $handle );
lukasbestle commented 1 year ago

The question is if the Response class should directly access the Request headers to determine the correct behavior or if it should be told which mode to use.

distantnative commented 1 year ago

@lukasbestle the easier way would probably yo do it within the/a Response class - just because otherwise we'd have to check quite a few spots in the code where the Response class would need to be told from outside.

lukasbestle commented 1 year ago

Easiest, definitely. But won't we shoot ourselves in the foot by attaching the two classes so closely together? Especially because we need to get "the" request object from the app instance.

Maybe we could have a string|false|null $range = null argument. If passed a string, it will use that range. If passed false, it will always output the full file. For null it will lazily try to access the app request object. If that's not there or the Range header does not exist, it will fall back to the full file.

distantnative commented 1 year ago

I started something https://github.com/getkirby/kirby/pull/5153 Would love to hear from anyone who knows a bit about range requests 🙏