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

Video Upload #655

Closed
wants to merge 1 commit into from
Closed

Video Upload #655

wants to merge 1 commit into from

Conversation

Choko256
Copy link

Make tweepy handle MP4 videos upload according to the Twitter API documentation for chunked uploads :
https://dev.twitter.com/rest/public/uploading-media#chunkedupload

@obskyr
Copy link
Contributor

obskyr commented Oct 3, 2015

I was going to work on this myself, but as it turns out, someone already had. Nice!

This doesn't seem to be usable, though. Sure, this allows for uploading videos, but there's no way to actually tweet them. The update_with_media (or really the update_status method, since update_with_media is deprecated) needs to be able to handle video files / filenames for it to actually be usable, right?

@Choko256
Copy link
Author

Choko256 commented Oct 3, 2015

Hello @obskyr !
No need to update update_status function. You can send the video link in the media_ids argument and it rocks 👍
I tried this last week and it works really fine ! Of course you will have to send Twitter-compliant videos (MP4 <= 30sec, 15MB max, 1280x1024max, etc. you can have the list of constraints in the given link). Otherwise, a TweepError is raised.

I did not change anything else and it works fine for me :)
Let me know if something goes wrong ;)

@Choko256
Copy link
Author

Choko256 commented Oct 3, 2015

Tip: You cannot send videos and images in a tweet. You can only send 1 video or 4 images max.

@obskyr
Copy link
Contributor

obskyr commented Oct 3, 2015

Oh, the docs don't mention the media_ids parameter. The docs are out of date in general, in fact. Cool, so there is a way to use it.

Regardless, don't you think it should be easier to tweet videos with the API? It'd be way more dev-friendly if you could just tweet a video with a filename / file object and it'd handle the upload under the hood for you, like what is done with images. All that would be needed would be some checks to see whether a file is a video or an image, which is easy (basically just check the extension).

@Choko256
Copy link
Author

Choko256 commented Oct 3, 2015

Yes you're right !
I will create a wrapper function to send directly tweets with medias by handling upload automatically ;)

Thanks!

@askaliuk
Copy link

Thank you for awesome work, @Choko256.
@joshthecoder do you plan to merge it?

@askaliuk askaliuk mentioned this pull request Oct 29, 2015
@jeremylow
Copy link

Hey, great work. I was just trying to find a solution to a GIF problem and ended up here. One thing that I found (since my code is python3): you're going to run into problems with the urllib import statement and the supported versions of python. Maybe change from:

import urllib

to:

if six.PY2:
    from urllib import urlencode
elif six.PY3:
    from urllib.parse import urlencode

And then change lines 1444 & 1481 to just use urlencode.

@askaliuk
Copy link

askaliuk commented Nov 4, 2015

Fails sometimes for me with following exception:

TweepError: {"request":"\/1.1\/media\/upload.json","error":"SegmentIndex must be between 0 and 999."}
  File "tweepy/api.py", line 252, in video_upload
    )(*args, **kwargs)
  File "tweepy/binder.py", line 243, in _call
    return method.execute()
  File "tweepy/binder.py", line 227, in execute
    raise TweepError(error_msg, resp)

@jeremylow
Copy link

How large is the file you're trying to upload

@askaliuk
Copy link

askaliuk commented Nov 4, 2015

Found root cause:
https://github.com/tweepy/tweepy/pull/655/files#diff-ea5dd38a4efd9ff36c96e04ab0597cfbR239

File size = 5219933.
chunk_size = 4096
nloops = 1275

I think the code should raise an exception if nloops > 999 and don't try to upload. Also explain to the method caller, that he/she needs to increase chunk_size or decrease file size.

@jeremylow
Copy link

4k is too small as a default. Should probably be set to something more reasonable like 1MB.

Rather than returning an error, if the user really wants a small chunk size, disallow any value smaller than 16K, so that it always succeeds. (Max size / 16K = 960 chunks)

@fitnr
Copy link
Contributor

fitnr commented Nov 10, 2015

This is great and I would love to see it in Tweepy asap.

But, what do you think about having a single media_upload function that's agnostic about file type? I think it would just need to have a single mimetypes.guess_type check, and then send the sile to the correct function. This would save trouble of having to check file file for applications that might be upoading video and images.

I mocked up these ideas (along with the change from @jeremylow and a Parser fix) here: fitnr@ca7fe53

@fitnr fitnr mentioned this pull request Nov 13, 2015
jeremylow referenced this pull request in fitnr/tweepy Nov 13, 2015
* And safe import urllib in both py2/3

* rename methods to match API
* DRY max sizes, put in class in case twitter changes them
* use single media_upload for image and video, send to standard or
* chunked upload based on mime type and size

* finalize needs ModelParser, not RawParser
LOZORD added a commit to jw-mcgrath/illumeme that referenced this pull request Apr 16, 2016
@jason-hyland-heyhuman

This comment has been minimized.

@beaumartinez

This comment has been minimized.

@ghost

This comment has been minimized.

@ghost

This comment has been minimized.

@IoIxD IoIxD mentioned this pull request Mar 11, 2019
@Harmon758 Harmon758 added the Feature This is regarding a new feature label Apr 28, 2019
@@ -211,6 +212,60 @@ def media_upload(self, filename, *args, **kwargs):
upload_api=True
)(*args, **kwargs)

def video_upload(self, filename, *args, **kwargs):
""" :reference https://dev.twitter.com/rest/reference/post/media/upload-chunked

This comment was marked as outdated.

@Harmon758 Harmon758 force-pushed the master branch 5 times, most recently from cf54e31 to d538993 Compare August 27, 2019 04:24
@Harmon758
Copy link
Member

Superseded by #672 and #929

@jason-hyland-heyhuman API.update_status does not accept filename or file parameters, and those aren't being added by this PR.

@Harmon758 Harmon758 closed this Sep 2, 2019
@Harmon758 Harmon758 added the Stale This is inactive, outdated, too old, or no longer applicable label Sep 2, 2019
@marvic2409

This comment has been minimized.

@Harmon758
Copy link
Member

The video-upload branch / pull request #1486 should be complete now.
Any feedback or review would be appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature This is regarding a new feature Stale This is inactive, outdated, too old, or no longer applicable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants