Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: youtube base.js parsing logic #1217

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

khlevon
Copy link
Contributor

@khlevon khlevon commented Apr 27, 2023

Summary

Youtube again changed the base.js code for more details, you can check this comment

To have stable lib, I suggest approving this refactoring of lib/sig.js file, which was done by @distubejs, and reviewed and improved by me.

Results

Fixed issue: #1216

Code tested by me on my own product, also I updated outdated tests.
You can use this patch until we make a decision regarding this PR

"ytdl-core": "git+ssh://git@github.com:khlevon/node-ytdl-core.git#v4.11.4-patch.2"

@khlevon
Copy link
Contributor Author

khlevon commented Apr 27, 2023

will check the test cases tomorrow

@khlevon
Copy link
Contributor Author

khlevon commented Apr 28, 2023

@fent please have a look at this PR, and if you agree, please let's merge it :)

@ShantanuNair
Copy link

@khlevon Any idea if there are any blockers on merging this? Appreciate the PR!

@khlevon
Copy link
Contributor Author

khlevon commented May 18, 2023

@ShantanuNair
I don't see any blocker. We need someone with merge access to review, approve and merge this PR.

@Salientekill
Copy link

Would you be able to make a version available for me to download and test?

@dani-mp
Copy link

dani-mp commented Sep 5, 2023

@fent it'd be great if you could give @khlevon access to the repository so he can help with the development!

@gmagreti
Copy link

gmagreti commented Sep 7, 2023

Você poderia disponibilizar uma versão para eu baixar e testar?

you can use this version while PR is open:

"ytdl-core": "git+ssh://git@github.com:khlevon/node-ytdl-core.git#v4.11.4-patch.2"

lovegaoshi added a commit to lovegaoshi/node-ytdl-core that referenced this pull request Sep 7, 2023
@helloworld9912
Copy link

Can you merge this PR ? last versions are broken

@dani-mp
Copy link

dani-mp commented Sep 9, 2023

@khlevon the patch is not working anymore, right?

@stedel
Copy link

stedel commented Sep 10, 2023

@khlevon the patch is not working anymore, right?

its still working in my app as of yesterday

@dani-mp
Copy link

dani-mp commented Sep 11, 2023

@stedel I'm glad to read that. Mine is getting this error since last Wednesday using the patch mentioned above:

2023-09-11T08:33:15.805Z	097f75ee-1b9a-40bc-aaa2-387d1577a90c	ERROR	evalmachine.<anonymous>:426
Ama=function(a,b,c,d){var e=null;switch(b){case "JSON":try{var f=c.responseText}catch(h){throw d=Error("Error reading responseText"),d.params=a,oC(d),h;}a=c.getResponseHeader("Content-Type")||"";f&&0<=a.indexOf("json")&&(")]};Ila(ncode);
                                                                                                                                                                                                                             ^^^^^^^^^^^^^^^^

SyntaxError: Invalid or unexpected token
    at new Script (node:vm:100:7)
    at Object.exports2.decipherFormats (/var/task/build/server.cjs:1013223:55)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Promise.all (index 0)
    at exports2.getInfo (/var/task/build/server.cjs:1013525:21)

@libhide
Copy link

libhide commented Nov 15, 2023

Can confirm that this patch does fix #1261 for me. Hoping this gets merged soon!

FoxxMD added a commit to FoxxMD/muse that referenced this pull request Mar 13, 2024
* patch-package replaces using git branch for ytdl-core
  * Need to use postinstall-postinstall because of yarn https://github.com/ds300/patch-package?tab=readme-ov-file#why-use-postinstall-postinstall-with-yarn
* Saves ~100MB on Docker image layer by dropping git dependency
* Can be removed once fent/node-ytdl-core#1217 is merged
@twarped
Copy link

twarped commented Apr 11, 2024

I say if @fent doesn't get back to us in three months, we have someone who knows what they're doing fork this repo and maintain it. There's just a lot of people wanting to contribute that have great ideas, and this repo might die if we don't do anything about it. There just aren't that many great ytdl's out there.

@FurblandChannel
Copy link

I say if @fent doesn't get back to us in three months, we have someone who knows what they're doing fork this repo and maintain it. There's just a lot of people wanting to contribute that have great ideas, and this repo might die if we don't do anything about it. There just aren't that many great ytdl's out there.

I second this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet