Sound preference proposal, closes #83 #84

smarek · 2019-04-07T11:51:49Z

No description provided.

ubershmekel · 2019-05-31T18:41:44Z

Thank you for doing this work! Two requests if you have time @smarek:

Seems to get v.reddit sounds to work we'd need to handle the hls_url instead of the fallback url. That's by looking at https://www.reddit.com/r/WhatsWrongWithYourDog/comments/bv7x95/so_our_dog_gets_a_bit_excited_when_asked_about/.json and https://www.reddit.com/r/WhatsWrongWithYourDog/comments/bv7x95/so_our_dog_gets_a_bit_excited_when_asked_about/
The buttons in the UI are getting too numerous and unwieldy. Something needs to be collapsed there perhaps. I also think I misplaced the comments and image links. All the checkboxes should be in one row. I don't expect you to to do that in this PR. Just saying I might have to do it if you fix the v.reddit issue.

smarek · 2019-06-07T10:43:45Z

fallback_url links directly the video, hls_url links m3u8 playlist, which is usually not very compatible with browsers, and I think we would have to integrate additional JS library to handle such playlists.
Do you insist on replacing fallback_url with hls_url ?

This is what I see in media.reddit_video. block of JSON url you've linked, maybe the contents changes over time, and you've seen something very different?

"media": {
"reddit_video": {
"fallback_url": "https://v.redd.it/3fqsu5bczj131/DASH_720?source=fallback",
"height": 720,
"width": 1280,
"scrubber_media_url": "https://v.redd.it/3fqsu5bczj131/DASH_96",
"dash_url": "https://v.redd.it/3fqsu5bczj131/DASHPlaylist.mpd",
"duration": 61,
"hls_url": "https://v.redd.it/3fqsu5bczj131/HLSPlaylist.m3u8",
"is_gif": false,
"transcoding_status": "completed"
}
},

yes, the layout is not very nice, and i've got no idea what to do with it, however that is something I can fix easily, once we agree on what design we want to have

I propose this controls layout:

subreddit link | nswf | sound | fullscreen
hotkeys | src | blog | comments | image
auto next form seconds
pagination

what do you think?

ubershmekel · 2019-06-08T02:29:07Z

I'm not sure I understand your proposal. https://v.redd.it/3fqsu5bczj131/DASH_720?source=fallback has no audio as far as I can tell. Do you hear audio at that url?
For layout, I'm thinking of separating the types of UI. First row would be links, the second would be the checkboxes.

redditp-info | subreddit link | comments | image
nsfw | sound | auto next | duration | fullscreen
pagination

Then redditp-info would link to the github readme. This would remove hotkeys | src | blog which would be explained and linked at the github readme.

What do you think of that layout?

smarek · 2019-06-08T07:19:12Z

js/script.js

@@ -334,6 +354,7 @@ $(function () {
 // Need to redesign this redditp thing.
 pic.type = imageTypes.gifv;
 pic.url = pic.data.media.reddit_video.fallback_url;
+ pic.sound = pic.url.substring(0,pic.url.lastIndexOf('/'))+"/audio";


smarek · 2019-06-08T07:24:52Z

sorry, audio is available at fallback_url/audio see the script.js line 357
No audio is on the media links, from the dog link you posted, if you look in network trace, you can see reddit is loading DASH video of specific quality (720p) and audio separately
I like it, my proposal tried to keep some things where they were, but if you're cool with complete reorg, i like it :)

ubershmekel · 2019-06-23T06:00:08Z

This works on v.redd.it urls like https://www.reddit.com/r/WhatsWrongWithYourDog/comments/c3x7cc/doggo_thinks_that_he_is_one_of_them/.json

Does not work on gfycat urls like https://www.reddit.com/r/nextfuckinglevel/comments/c3v7b3/looks_so_effortless_but_he_must_have_trained_very/.json

…-sound2 # Conflicts: # index.html # js/EmbedIt.js # js/script.js Note this only seems to work for video URLs that have sound in them. Not for recombining separate sound file with video file (like reddit). So it only works for imgur.

ubershmekel · 2020-02-06T07:49:57Z

I re-merged and tested this. This works for a file that has video+audio in it, but not for separate video and audio files like reddit videos. Seems only imgur and gfycat will work with this.

I think to make it fully work we'd need to implement something like this JS multiplexing. Which is what I assume reddit video has done: https://medium.com/canal-tech/how-video-streaming-works-on-the-web-an-introduction-7919739f7e1

I'm not sure if we should merge it in when reddit videos don't work.

smarek · 2020-02-06T14:09:08Z

Could you maybe create a branch on which we could propose PRs to fix remaining issues with sound, from mentioned "re-merged" branch you already have? Or if you have staging environment somewhere for others to test the sound with (if not, it can be disabled by default and listed under experimental options)

Also the "multiplexing" is imho not needed, as you can simply provide different URL that contains MP4 container (audio+video) , or provide separate <video and <audio html tags and sync them using https://stackoverflow.com/a/32323192/492624 or similar techniques

ubershmekel · 2020-02-06T17:23:24Z

https://github.com/ubershmekel/redditp/commits/smarek-sound2

You're right. Making separate elements would work too.

smarek · 2020-02-10T13:44:22Z

@ubershmekel before i start working on that, would you be cool with using dash.js for playing (json)reddit_video.dash_url instead of using plain html5 elements?

ubershmekel · 2020-02-10T17:10:31Z

I see the minified file is 577 KB (non-gzipped size). I couldn't find a browser compatibility matrix. Can you explain the cost-benefit? Seems a bit heavy and potentially fragile.

smarek · 2020-02-10T18:08:12Z

Given raw-json

"reddit_video": {
"fallback_url": "https://v.redd.it/iysd6l20k3941/DASH_480?source=fallback",
"height": 480,
"width": 384,
"scrubber_media_url": "https://v.redd.it/iysd6l20k3941/DASH_96",
"dash_url": "https://v.redd.it/iysd6l20k3941/DASHPlaylist.mpd",
"duration": 19,
"hls_url": "https://v.redd.it/iysd6l20k3941/HLSPlaylist.m3u8",
"is_gif": false,
"transcoding_status": "completed"
}

We can either do

Plain html+js (whatever source data)
dash.js (dash_url)
hls.js (hls_url)
???
(maybe we can do all of them, and fall-back through when given browser does not support one but supports other?)

I've found conflicted sources, but it seems to me that HLS, being Apple technology, has far more lower support than DASH

However reddit itself (on my device) opens with HLS source

<video poster="https://external-preview.redd.it/SZJwCdXt4knnZgskQuE2_C_QDyCXQeDNReuwC-lp5aw.png?width=320&amp;crop=smart&amp;format=pjpg&amp;auto=webp&amp;s=53e709f966bf326c52cb3b64b995cdb2525e2bd8" muted="" preload="auto" class="_1EQJpXY7ExS04odI1YBBlj" src="blob:https://www.reddit.com/b59eb125-a56e-46b4-9d40-9bd09d38b4c4">
<source src="https://v.redd.it/iysd6l20k3941/HLSPlaylist.m3u8" type="application/vnd.apple.mpegURL">
</video>

I'd be really grateful if someone could investigate the protocol compatibility between HLS/DASH

Relevant tickets

Check dash.js browser support Dash-Industry-Forum/dash.js#2055

…xed getNextSlideIndex when no non-nsfw post is available up in queue

…page / comments link

comment links do not contain CORS headers, so it must be loaded via jsonp to bypass the security measures /r/randnsfw, /r/random responses with redirect, and thus must be loaded as jsonp see docs https://www.reddit.com/wiki/random

…external libraries

smarek · 2020-02-27T10:51:52Z

@ubershmekel I'm quite sorry, but I've solved this ticket and related #104 ticket, but the PR is quite a lot changes, because I was not able to develop efficiently, with so much code issues.

Changes i've included along (and the PR is currently mergable without conflicts against master)

ignoring idea (idea-community/webstorm/phpstorm) dev files
code formatting, terminating statements with ;, strict comparison in if statements
ability to handle single-post URL (useful for debugging)
fixed distinguish for json/jsonp requests
disable cloud reporting on non-production environment

I've left the commits un-squashed, but after review, if accepted, i can (or you can) squash some of them together

Also, since I've updated this PR and did not create new one against branch smarek-sound2, said branch can be deleted currently

smarek · 2020-02-27T10:57:53Z

Cool, deploy preview works on my end correctly in all test-cases, check the links below:

https://deploy-preview-84--redditp.netlify.com/
https://deploy-preview-84--redditp.netlify.com/r/all
https://deploy-preview-84--redditp.netlify.com/r/WhatsWrongWithYourDog/comments/c3x7cc/doggo_thinks_that_he_is_one_of_them
https://deploy-preview-84--redditp.netlify.com/r/nextfuckinglevel/comments/c3v7b3/looks_so_effortless_but_he_must_have_trained_very
https://deploy-preview-84--redditp.netlify.com/r/random
https://deploy-preview-84--redditp.netlify.com/r/randnsfw
https://deploy-preview-84--redditp.netlify.com/

smarek · 2020-02-27T11:10:47Z

js/script.js

@@ -435,7 +464,8 @@ $(function () {
 $('#titleDiv .collapser').click();
 break;
 case A_KEY:
- $("#autoNextSlide").prop("checked", !$("#autoNextSlide").is(':checked'));
+ var $ans = $("#autoNextSlide");
+ $ans.prop("checked", !$ans.is(':checked'));


this is lint issue duplicate jquery selector

smarek · 2020-02-27T11:11:31Z

js/script.js

 if (!rp.photos[i].over18) {
 return i;
 }
 }
+ return 0;


here not handling situation where none of available pictures is non-nsfw caused ajax looping, but it's obscure usecase

index.html

smarek · 2020-02-27T11:13:59Z

js/script.js

+ if (videoTags.length === 1) {
+ videoTags[0].muted = !rp.settings.sound;
+ }
+ var audioTags = document.getElementsByTagName('audio');


since for v.redd.it audio tag is separate from video tag, this needs to be checked as well

smarek · 2020-02-27T11:14:26Z

js/script.js

@@ -342,14 +370,15 @@ $(function () {
 // some crossposts don't have a pic.data.media obj?
 return;
 }
+ pic.sound = pic.url.substring(0, pic.url.lastIndexOf('/')) + "/audio";


this generates audio link for v.redd.it links only

smarek · 2020-02-27T11:15:32Z

js/script.js

- //log("skipped bad extension: " + url);
- return false;
- }
+ return rp.settings.goodExtensions.indexOf(extension) >= 0;


simplify expression lint

smarek · 2020-02-27T11:16:25Z

js/script.js

- return false;
- }
- }
+ return !!query.shuffle;


bang-bang, see https://gist.github.com/mledour/d936fa347a8c3d754c03f2ad8cd96c88

smarek · 2020-02-27T11:16:49Z

js/script.js

-
+ };
+
+ var getStackTrace = function () {


helper function, unused currently, but very useful in tracing dev issues

smarek · 2020-02-27T11:17:18Z

js/script.js

@@ -811,7 +850,8 @@ $(function () {
 rp.session.loadingNextImages = true;

 // Note that JSONP requests require `".json?jsonp=?"` here.
- var jsonUrl = rp.redditBaseUrl + rp.subredditUrl + ".json?jsonp=?" + rp.session.after + "&" + getVars;
+ var jsonUrl = rp.redditBaseUrl + rp.subredditUrl + ".json?" + (rp.session.after ? rp.session.after + "&" : "") + getVars;


jsonp query attribute is added based on below useJsonP classification

ubershmekel · 2020-02-28T20:09:38Z

Looks like a lot of work. What's the state of affairs?

smarek · 2020-02-28T20:10:35Z

What do you mean?

Co-Authored-By: Yuval Greenfield <[email protected]>

smarek · 2020-03-01T08:07:45Z

If the single proposed change is everything, i think, it's ready to merge

ubershmekel · 2020-03-01T10:34:31Z

Looking at the code, this is all great stuff. I wish I had a test suite that could run through more scenarios. I hope to finish reviewing tomorrow.

smarek · 2020-03-01T10:59:11Z

Only thing I'm not satisfied with, is the detection of subreddits, that redirect.

There are certainly more than what's hard-coded currently, and I did not find any official list of this kind of subreddits, and detecting whether the call resulted in response from different subreddit (which might not be complete/best solution), is not possible via standard jQuery XHR means (currently being solved here jquery/jquery#4405 )

I propose to solve this in subsequent PR, because ie. r/random returns data from randomly picked subreddit, but currently implementation calls next page of items from url such as http://www.reddit.com/r/random.json?&after=t3_fap561& which is not correct obviously

ubershmekel · 2020-03-02T06:33:05Z

js/script.js

- // is the current solution sadly.
- // Note we're still using `jsonp` despite potential issues because
- // `http://www.redditp.com/r/randnsfw` wasn't working with CORS for some reason.
- // https://github.com/ubershmekel/redditp/issues/104


You removed this comment, without providing an explanation for the jsonp toggling or why did you make the following specific exceptions touseJsonP. We should pick this up in a future PR.

Sure, it seemed to me that useJsonP with the code, that decides whether it's true or false, is pretty self-explanatory, but as I said, I expect this to be solved better when we tackle the redirected subreddits.

ubershmekel · 2020-03-02T06:34:16Z

I love that getting a /comments/ URL works now. I was hoping to get that fixed some day. I'm curious why you need to toggle between jsonp and not for specific URLs. I thought JSONP works for everything. No?

Merging. We'll get the rest sorted later. Thank you so much for all this work @smarek it works beautifully as far as I can tell.

smarek · 2020-03-02T06:55:11Z

I love that getting a /comments/ URL works now. I was hoping to get that fixed some day. I'm curious why you need to toggle between jsonp and not for specific URLs. I thought JSONP works for everything. No?

This is given by response type of the endpoint, when the request is redirected or single-post, the response type does not match application/json, and is blocked by CORB (at least in my chrome dev version 82)

Anyway, I'm glad you like the work, and you should probably delete the smarek-sound2 branch now

Sound preference proposal, adding audio track on reddit videos

9125aef

smarek mentioned this pull request Apr 10, 2019

Sound enable/disable preference #83

Closed

smarek commented Jun 8, 2019

View reviewed changes

ubershmekel mentioned this pull request Jun 16, 2019

Added animation settings #87

Open

smarek added 9 commits February 27, 2020 09:24

idea/phpstorm development

8e6e941

Merge remote-tracking branch 'upstream/master' into smarek-sound2

b507b29

formatted code, terminated function/var statements with semicolon, fi…

7b95f0d

…xed getNextSlideIndex when no non-nsfw post is available up in queue

strict comparison in if/else statements

366a942

handle single page links, use plain json for ajax requests on single …

dc30c2a

…page / comments link

disable cloud reporting on non-production environments

49083b5

Closes #104

113c05b

comment links do not contain CORS headers, so it must be loaded via jsonp to bypass the security measures /r/randnsfw, /r/random responses with redirect, and thus must be loaded as jsonp see docs https://www.reddit.com/wiki/random

reverted to not show default html video tag controls

94a39bf

solved reddit-video (v.redd.it) audio+video synchronization using no …

679e16c

…external libraries

smarek commented Feb 27, 2020

View reviewed changes

index.html Outdated Show resolved Hide resolved

smarek commented Feb 27, 2020

View reviewed changes

js/script.js

};

var getStackTrace = function () {

Copy link

Contributor Author

smarek Feb 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

helper function, unused currently, but very useful in tracing dev issues

smarek commented Feb 27, 2020

View reviewed changes

Update index.html

993f24b

Co-Authored-By: Yuval Greenfield <[email protected]>

ubershmekel reviewed Mar 2, 2020

View reviewed changes

ubershmekel merged commit a060603 into ubershmekel:master Mar 2, 2020

smarek deleted the sound branch March 2, 2020 06:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sound preference proposal, closes #83 #84

Sound preference proposal, closes #83 #84

smarek commented Apr 7, 2019

ubershmekel commented May 31, 2019 •

edited

smarek commented Jun 7, 2019 •

edited

ubershmekel commented Jun 8, 2019

smarek Jun 8, 2019

smarek commented Jun 8, 2019 •

edited

ubershmekel commented Jun 23, 2019

ubershmekel commented Feb 6, 2020

smarek commented Feb 6, 2020

ubershmekel commented Feb 6, 2020 •

edited

smarek commented Feb 10, 2020

ubershmekel commented Feb 10, 2020

smarek commented Feb 10, 2020

smarek commented Feb 27, 2020

smarek commented Feb 27, 2020

smarek Feb 27, 2020

smarek Feb 27, 2020

smarek Feb 27, 2020

smarek Feb 27, 2020

smarek Feb 27, 2020

smarek Feb 27, 2020

smarek Feb 27, 2020

smarek Feb 27, 2020

ubershmekel commented Feb 28, 2020

smarek commented Feb 28, 2020

smarek commented Mar 1, 2020

ubershmekel commented Mar 1, 2020

smarek commented Mar 1, 2020

ubershmekel Mar 2, 2020

smarek Mar 2, 2020

ubershmekel commented Mar 2, 2020

smarek commented Mar 2, 2020 •

edited

Sound preference proposal, closes #83 #84

Sound preference proposal, closes #83 #84

Conversation

smarek commented Apr 7, 2019

ubershmekel commented May 31, 2019 • edited

smarek commented Jun 7, 2019 • edited

ubershmekel commented Jun 8, 2019

Choose a reason for hiding this comment

smarek commented Jun 8, 2019 • edited

ubershmekel commented Jun 23, 2019

ubershmekel commented Feb 6, 2020

smarek commented Feb 6, 2020

ubershmekel commented Feb 6, 2020 • edited

smarek commented Feb 10, 2020

ubershmekel commented Feb 10, 2020

smarek commented Feb 10, 2020

smarek commented Feb 27, 2020

smarek commented Feb 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ubershmekel commented Feb 28, 2020

smarek commented Feb 28, 2020

smarek commented Mar 1, 2020

ubershmekel commented Mar 1, 2020

smarek commented Mar 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ubershmekel commented Mar 2, 2020

smarek commented Mar 2, 2020 • edited

ubershmekel commented May 31, 2019 •

edited

smarek commented Jun 7, 2019 •

edited

smarek commented Jun 8, 2019 •

edited

ubershmekel commented Feb 6, 2020 •

edited

smarek commented Mar 2, 2020 •

edited