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

[ie/afreecatv] Fix extractor #9566

Merged
merged 9 commits into from Apr 6, 2024

Conversation

Tomoka1
Copy link
Contributor

@Tomoka1 Tomoka1 commented Mar 29, 2024

IMPORTANT: PRs without the template will be CLOSED

Description of your pull request and other information

As in a somewhat reversal of #6283 this uses fully JSON again as some necessary information are not provided in XML any more.
This code will not keep XML part for backward compatibility.
The code is tested with login and without, on adult and regular.

Closes #4592, Closes #8862, Closes #9544

Template

Before submitting a pull request make sure you have:

In order to be accepted and merged into yt-dlp each piece of code must be in public domain or released under Unlicense. Check all of the following options that apply:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

@bashonly bashonly added the site-bug Issue with a specific website label Mar 30, 2024
@zyz-1998
Copy link

still is problem : ERROR: [afreecatv] 109615861: 109615861: Failed to parse XML (caused by ParseError

@Tomoka1
Copy link
Contributor Author

Tomoka1 commented Mar 30, 2024

still is problem : ERROR: [afreecatv] 109615861: 109615861: Failed to parse XML (caused by ParseError

Are you sure you used the suggested changes? There shouldn't be anything to parse XML left.
Your example 109615861 works fine for me.

@zyz-1998
Copy link

I deployed yt_dlp using
C:\Users\ZYZ.conda\envs\python3.8\python.exe -m pip install --force-reinstall "yt-dlp[default] @ https://github.com/yt-dlp/yt- dlp/archive/master.tar.gz"

Is it because the latest package was not downloaded? Thank you.

@HobbyistDev
Copy link
Contributor

HobbyistDev commented Mar 30, 2024

I deployed yt_dlp using
C:\Users\ZYZ.conda\envs\python3.8\python.exe -m pip install --force-reinstall "yt-dlp[default] @ https://github.com/yt-dlp/yt-dlp/archive/master.tar.gz"
Is it because the latest package was not downloaded? Thank you.

I think you should get yt-dlp from Tomoka1's branch first (because it's not in main branch yet)


While the aim of the PR is not to extract XML file, i think it nice if we could extract the upload_date (and perhaps other key) like #8862

@zyz-1998
Copy link

What should be done, thank you

@zyz-1998
Copy link

new probelm ERROR: [afreecatv] 109615861: 109615861: HTTP Error 502: Bad Gateway (caused by <HTTPError 502: Bad Gateway>)

@Tomoka1
Copy link
Contributor Author

Tomoka1 commented Mar 30, 2024

HobbyistDev wrote:

While the aim of the PR is not to extract XML file, i think it nice if we could extract the upload_date (and perhaps other key) like #8862

I am not sure what you're referring to.
upload_date is retrieved from file_info_key as it was done before in similar fashion.
Using write_tm or broad_start wouldn't be an issue, if that's preferred.

[~]$ python -m yt_dlp https://vod.afreecatv.com/player/109615861 --print "%(upload_date)s %(title)s" 
20231116 [클립]생)사랑스런 민영이 오늘은 시스루~ 검스 ㅎ ㅏㅏㅏ ㅀ

zyz-1998 wrote:

new probelm ERROR: [afreecatv] 109615861: 109615861: HTTP Error 502: Bad Gateway (caused by <HTTPError 502: Bad Gateway>)

Not replicable on my side. I assume you're sitting in some geolocked region.
Try curl https://api.m.afreecatv.com/station/video/a/view and if it's negative please open another issue.

Copy link
Member

@bashonly bashonly left a comment

Choose a reason for hiding this comment

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

Partial review

We don't need the for _ in range(2): loop at all anymore, right? IIRC it was used to re-fetch adult content by updating the query for the XML request. But we don't do that anymore.

Can you also update the tests and add an adult content test, since the current one is dead?

yt_dlp/extractor/afreecatv.py Outdated Show resolved Hide resolved
yt_dlp/extractor/afreecatv.py Outdated Show resolved Hide resolved
@bashonly bashonly added the pending-fixes PR has had changes requested label Apr 2, 2024
@bashonly
Copy link
Member

bashonly commented Apr 2, 2024

I removed all the dead code and simplified/modernized what was left. Please test my changes and let me know what you think.

My previous review is no longer applicable except for this part:

Can you ... add an adult content test, since the current one is dead?

I've added an only_matching test for an adult content URL, it would be nice if you could test it / convert it into a proper test

@bashonly bashonly added needs-testing Patch needs testing and removed pending-fixes PR has had changes requested labels Apr 2, 2024
@bashonly bashonly self-assigned this Apr 3, 2024
@bashonly bashonly changed the title [extractor/afreecatv] Fix extractor [ie/afreecatv] Fix extractor Apr 4, 2024
@bashonly bashonly added pending-review PR needs a review and removed needs-testing Patch needs testing pending-review PR needs a review labels Apr 5, 2024
@bashonly bashonly merged commit 9415f1a into yt-dlp:master Apr 6, 2024
6 checks passed
@Tomoka1
Copy link
Contributor Author

Tomoka1 commented Apr 7, 2024

@bashonly Oh, i just wrote up-to-date tests myself before seeing the merge. Sorry i didn't have too much time this week. Thanks for your hard work!

aalsuwaidi pushed a commit to aalsuwaidi/yt-dlp that referenced this pull request Apr 21, 2024
Closes yt-dlp#4592, Closes yt-dlp#8862, Closes yt-dlp#9544
Authored by: bashonly, Tomoka1

Co-authored-by: bashonly <88596187+bashonly@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
site-bug Issue with a specific website
Projects
None yet
4 participants