VID-14: Browser tab will lock up if video is not HLS and mimetype omitted

Issue Information

Issue Type: Bug
 
Priority: Major
Status: Closed

Reported By:
Ben Tasker
Assigned To:
Ben Tasker
Project: Videos.bentasker.co.uk (VID)
Resolution: Fixed (2019-12-15 11:13:49)
Affects Version: v0.11, v0.12, v0.13, v0.14,
Target version: v0.15,
Components: Embed Code ,

Created: 2019-04-25 15:34:44
Time Spent Working


Description
The Player page Player.html is fairly simplistic, extracting a video path from the query string in order to pass into this statement 
<script type="text/javascript">embedBensPlayer(vidurl);</script>


However, if the video is an MP4 (for example) rather than being HLS, the player will instantiate and then that browser tab will become unresponsive in Chromium.

This is because the embed script assumes HLS by default, and so will have written a source tag in with the type attribute set to application/x-mpegURL.

The player then tries to parse the blob as if it were a manifest.


Toggle State Changes

Activity


2019-04-25 15:37:08
Fixing in the player page would probably be reasonably trivial, would just need to check the extension of the provided player path and then inject a mimetype based upon that.

However, this issue isn't really constrained to the Player page (that's just where it was detected), and an embed anywhere could trigger it if the mimetype were excluded from the call to embedBensPlayer.

So really, the embed script itself needs updating to do some form of mimetype detection (possibly allowing override still?)

I'm going to update this issue title from being specific to the player to being more generic

2019-04-25 15:37:59
btasker added 'VNEXT' to Fix Version

2019-12-15 10:01:39
Repo: videos.bentasker.co.uk
Host:Rimmer

commit c2d7249fd148bb8ff7dcabf01581f70fded3e3c2
Author: B Tasker <github@<Domain Hidden>>
Date: Sun Dec 15 09:59:03 2019 +0000

Commit Message: VID-14 Calculate Mimetype when omitted from call to embedBensPlayer

Introduces new function guessMimeFromUrl which'll use the filename extension to try and derive a mime-type.

That currently only happens if the calling function hasn't specified a mime-type. Should probably think about whether we want to allow the calculation to be overridden or not (as allowing it still leaves potential for tab lockups).

If a mime-type cannot be calculated, then video/mp4 is used as the most common and (probably) least harmful default

resources/embed/embed.js | 76 +++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 75 insertions(+), 1 deletions(-)


View Commit | View Changes

2019-12-15 10:49:39
Repo: videos.bentasker.co.uk
Host:Rimmer

commit eff9e98d2b9fb15ca242bdc473f4ed96ec2e4697
Author: B Tasker <github@<Domain Hidden>>
Date: Sun Dec 15 10:47:05 2019 +0000

Commit Message: Add 2 test pages for VID-14

- The first uses the current release to repro the issue - failed playback and possibly a locked up browser tab
- The second uses vnext to test the fix, having the embed script guess the mimetype, allowing playback

Works as expected

resources/tests/vid_14_mimecalc_test.html | 17 +++++++++++++++++
resources/tests/vid_14_mimecalc_test_vnext.html | 17 +++++++++++++++++
2 files changed, 34 insertions(+), 0 deletions(-)


View Commit | View Changes

2019-12-15 10:53:27
Dumping out the extensions currently hosted, I think the case statement has pretty good coverage tbh 
ben@hiyori:~/videos$ find ./ -type f | grep -o -P '\.[a-z0-9]+$' | sort | uniq -c
      5 .css
      1 .editorconfig
      1 .gitignore
     35 .html
     70 .js
     64 .json
    108 .m3u8
      7 .md
      1 .mp3
      3 .mp4
     27 .nfo
      1 .npmignore
      1 .nvmrc
      1 .png
      1 .svg
   1805 .ts
      1 .ttf
      1 .txt
      8 .vtt
      1 .woff
      1 .yml
      1 .zip

Obviously some of those aren't videos and won't be called via the script.

The only other thing to decide, really, is whether or not we remove the ability to manually specify the mimetype. That'd prevent accidents breaking playback, but would mean that it's hard to quickly workaround a bug in the calculator.

I guess the ability to override could be added later, it'd need to be in a way that didn't mean all existing embeds override by default.

2019-12-15 10:55:39
Repo: videos.bentasker.co.uk
Host:Rimmer

commit 1660f8aa7093d65e658a680b4c08660c49d939eb
Author: B Tasker <github@<Domain Hidden>>
Date: Sun Dec 15 10:54:07 2019 +0000

Commit Message: VID-14 Ignore user specified mime-type and calculate it from the playurl

Existing embeds will continue to work as embedBensPlayer still accepts a second argument, it just no longer does anything with it

resources/embed/embed.js | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)


View Commit | View Changes

2019-12-15 10:59:39
Repo: videos.bentasker.co.uk
Host:Rimmer

commit ed3b96575fd8bd3c90e02688de7f0e4e6d414933
Author: B Tasker <github@<Domain Hidden>>
Date: Sun Dec 15 10:58:47 2019 +0000

Commit Message: Minify embed.js to include VID-14 changes in distribution

resources/embed/embed.min.js | 11 +----------
1 files changed, 1 insertions(+), 10 deletions(-)


View Commit | View Changes

2019-12-15 11:13:50
btasker changed status from 'Open' to 'Resolved'

2019-12-15 11:13:50
btasker added 'Fixed' to resolution

2019-12-15 11:13:53
btasker changed status from 'Resolved' to 'Closed'