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

Facebook info extractor #76

Closed
wants to merge 1 commit into from
Closed

Conversation

imrehg
Copy link
Contributor

@imrehg imrehg commented Feb 14, 2011

This is the first attempt on a FB info extractor. Tried to follow the other examples as close as possible.

Unfortunately I had to add one more import, if you know a way to unescape unicode-escaped strings (needed to extract video urls) with the current modules, I'd be glad to change it.

I'm also glad to have any other suggestions, I'm pretty sure you have advice in case I overcomplicated something (again).

This IE should be full-featured.

Public videos can be downloaded without login, e.g:
https://www.facebook.com/video/video.php?v=696729990595

Private videos need login, and subject to login rate limit of
a couple of tries / minute.
@rg3
Copy link
Collaborator

rg3 commented Feb 18, 2011

Hi,

Sorry for taking so long to answer. I've been a bit busy this week. The code looks good and no doubt I'll merge it as long as you commit to support the InfoExtractor. It seems to integrate very well with the existing code. I'll make a few comments so you can address possible issues.

It prints "Facebook" with a capital F. Normally we print the name all in lowercase, so you could change that if you are going to make more changes. It's a minor detail.

Second, there is a block of code in the InfoExtractor indented with 4 spaces instead of tabs. You could probably use tabs to be consistent with the rest of the code.

In that same loop, line 2394, you use the word "format" as a variable name. "format" is a reserved word in some Python versions and, while you can assign another value to it, it's dangerous because we could add code that uses it later and it wouldn't work.

http://docs.python.org/library/functions.html#format

Finally, I don't mind importing more modules if they allow for doing something easily, as long as the modules are available in Python 2.5. My worry here is that the function you're using, unicode_escape_decode, is not mentioned in the documentation. Could you investigate this a little bit? If you explain me what it's supposed to do with an example, maybe we can code a safer version of that routine.

Best regards.

@FelixChery FelixChery mentioned this pull request Apr 24, 2019
joedborg referenced this pull request in joedborg/youtube-dl Nov 17, 2020
[pull] master from rg3:master
This pull request was closed.
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

2 participants