MP4 video playback without JavaScript #634

Open
zedeus wants to merge 8 commits from feature/mp4-streaming into master
zedeus commented 2022-06-06 16:58:18 -04:00 (Migrated from github.com)
No description provided.
HookedBehemoth commented 2022-06-08 07:29:53 -04:00 (Migrated from github.com)

VIdeo streams work nicely with both ff and chromium.
The /pic/orig/ endpoint, as accessed through clicking on tweet images, is broken now though.
e.g. /pic/orig/media%2FFUufO3vX0AEmC95.png

VIdeo streams work nicely with both ff and chromium. The /pic/orig/ endpoint, as accessed through clicking on tweet images, is broken now though. e.g. /pic/orig/media%2FFUufO3vX0AEmC95.png
zedeus commented 2022-06-08 07:37:36 -04:00 (Migrated from github.com)

Oh, does it work if you swap the position of the orig and non-orig routes? I did it because I figured the order might have a tiny impact on performance, but didn't properly test it

Oh, does it work if you swap the position of the orig and non-orig routes? I did it because I figured the order might have a tiny impact on performance, but didn't properly test it
HookedBehemoth commented 2022-06-08 07:49:03 -04:00 (Migrated from github.com)

That works 👍🏻

That works 👍🏻
girst (Migrated from github.com) reviewed 2022-06-15 17:17:10 -04:00
girst (Migrated from github.com) commented 2022-06-15 17:17:09 -04:00
    playbackType = if video.hasMp4Url: mp4

to keep non-proxied videos working

```suggestion playbackType = if video.hasMp4Url: mp4 ``` to keep non-proxied videos working
zedeus (Migrated from github.com) reviewed 2022-06-15 19:56:18 -04:00
zedeus (Migrated from github.com) commented 2022-06-15 19:56:18 -04:00

They don't actually work every time. With the way Twitter serves mp4, a "cache miss" uses chunked transfer-encoding which results in 1 second playing and the video freezing until reload. That's what the retry logic is for in the media router, streaming the file without waiting for the cache is impossible.

They don't actually work every time. With the way Twitter serves mp4, a "cache miss" uses chunked transfer-encoding which results in 1 second playing and the video freezing until reload. That's what the retry logic is for in the media router, streaming the file without waiting for the cache is impossible.
girst (Migrated from github.com) reviewed 2022-06-16 10:17:13 -04:00
girst (Migrated from github.com) commented 2022-06-16 10:17:13 -04:00

If proxying videos is disabled, your change would make it impossible to play videos at all, since browsers can't play m3u8 without hls.js (which is bound by CORS, so it only works if videos are proxied). I never experienced the transfer-encoding bug you mentioned, but IMO having it fail some of the time is better than having it fail all the time.

If proxying videos is disabled, your change would make it impossible to play videos at all, since browsers can't play m3u8 without hls.js (which is bound by CORS, so it only works if videos are proxied). I never experienced the transfer-encoding bug you mentioned, but IMO having it fail some of the time is better than having it fail all the time.
zedeus (Migrated from github.com) reviewed 2022-06-16 12:44:11 -04:00
zedeus (Migrated from github.com) commented 2022-06-16 12:44:10 -04:00

Why would hls.js not be there? I tested this thoroughly, the transfer-encoding bug actually happens quite often both in the browser and when fetched via a headless client, it simply doesn't work. If you go to to the search page and filter native video it's fairly easy to hit a cache miss.

Why would hls.js not be there? I tested this thoroughly, the transfer-encoding bug actually happens quite often both in the browser and when fetched via a headless client, it simply doesn't work. If you go to to the search page and filter native video it's fairly easy to hit a cache miss.
girst (Migrated from github.com) reviewed 2022-06-16 13:26:19 -04:00
girst (Migrated from github.com) commented 2022-06-16 13:26:19 -04:00

Why would hls.js not be there?

it is, but it doesn't work when proxying is disabled (as far as i understand, it needs CORS headers to be sent from 3rd party domains (like video.twimg.com) in order for its requests to not get blocked by the browser).
but this is besides the point; I'd like to be able to see twitter videos even when javascript and proxying are disabled. and this works right now, and won't if this patch gets committed as-is.

I tested this thoroughly, the transfer-encoding bug actually happens quite often both in the browser and when fetched via a headless client, it simply doesn't work.
If you go to to the search page and filter native video it's fairly easy to hit a cache miss.

just tried that. these are the headers i get served from video files (not proxied): https://pastebin.com/yL485srS the first one is x-cache: MISS, the second one is x-cache: HIT. neither are sent using transfer-encoding:chunked and play fine. I went through multiple pages of /search?f=tweets&q=&f-native_video=on. I could reproduce this on my private instance (us-hosted) and nitter.net by disabling video proying and enabling mp4 playback with the same results.

I'm not saying you're wrong, but maybe we are talking about different things? maybe an alternative would be this:

    playbackType = if not prefs.hlsPlayback and video.hasMp4Url: mp4
> Why would hls.js not be there? it is, but it doesn't work when proxying is disabled (as far as i understand, it needs CORS headers to be sent from 3rd party domains (like video.twimg.com) in order for its requests to not get blocked by the browser). but this is besides the point; I'd like to be able to see twitter videos even when javascript _and_ proxying are disabled. and this works right now, and won't if this patch gets committed as-is. >I tested this thoroughly, the transfer-encoding bug actually happens quite often both in the browser and when fetched via a headless client, it simply doesn't work. > If you go to to the search page and filter native video it's fairly easy to hit a cache miss. just tried that. these are the headers i get served from video files (not proxied): https://pastebin.com/yL485srS the first one is `x-cache: MISS`, the second one is `x-cache: HIT`. neither are sent using transfer-encoding:chunked and play fine. I went through multiple pages of `/search?f=tweets&q=&f-native_video=on`. I could reproduce this on my private instance (us-hosted) and nitter.net by disabling video proying and enabling mp4 playback with the same results. I'm not saying you're wrong, but maybe we are talking about different things? maybe an alternative would be this: ```suggestion playbackType = if not prefs.hlsPlayback and video.hasMp4Url: mp4 ```
zedeus (Migrated from github.com) reviewed 2022-06-16 15:08:00 -04:00
zedeus (Migrated from github.com) commented 2022-06-16 15:08:00 -04:00

Maybe their US caching works in a different way than in the EU, dunno, but I know for a fact I ran into this chunked encoding issue which broke video playback a lot of times even while proxying through Nitter. It happened when I was working on it in 2019, and it happened a week ago when I worked on this version. I'd be ok with some sort of "Always stream MP4" preference disabled by default, but serving non-proxied mp4 from Twitter is too broken.

Maybe their US caching works in a different way than in the EU, dunno, but I know for a fact I ran into this chunked encoding issue which broke video playback a lot of times even while proxying through Nitter. It happened when I was working on it in 2019, and it happened a week ago when I worked on this version. I'd be ok with some sort of "Always stream MP4" preference disabled by default, but serving non-proxied mp4 from Twitter is too broken.
girst (Migrated from github.com) reviewed 2022-06-16 15:25:45 -04:00
girst (Migrated from github.com) commented 2022-06-16 15:25:45 -04:00

I'd be ok with some sort of "Always stream MP4" preference disabled by default,

that would be fine for my needs. thank you for considering that!

i also need to retract my earlier statement regarding hls.js requiring proxy: twitter actually sends the required cors-headers (access-control-allow-origin: *). i must have misremembered them not provididing those.

> I'd be ok with some sort of "Always stream MP4" preference disabled by default, that would be fine for my needs. thank you for considering that! i also need to retract my earlier statement regarding hls.js requiring proxy: twitter actually sends the required cors-headers (`access-control-allow-origin: *`). i must have misremembered them not provididing those.
This pull request has changes conflicting with the target branch.
  • src/parser.nim
  • src/routes/media.nim
  • src/views/renderutils.nim
  • src/views/tweet.nim
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feature/mp4-streaming:feature/mp4-streaming
git switch feature/mp4-streaming

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch master
git merge --no-ff feature/mp4-streaming
git switch feature/mp4-streaming
git rebase master
git switch master
git merge --ff-only feature/mp4-streaming
git switch feature/mp4-streaming
git rebase master
git switch master
git merge --no-ff feature/mp4-streaming
git switch master
git merge --squash feature/mp4-streaming
git switch master
git merge --ff-only feature/mp4-streaming
git switch master
git merge feature/mp4-streaming
git push origin master
Sign in to join this conversation.
No description provided.