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

Add support for replacing Vimeo video (Issue #55) #66

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

Conversation

michaelboccara
Copy link
Contributor

WIP: Partial fix for Issue #55

Add support for replacing existing video. But I get an error message from the Vimeo server

  • Added to VimeoRecorder Editor GUI
  • Added a replaceExisting boolean field to VimeoRecorder

Went according to documentation in:
https://developer.vimeo.com/api/upload/videos#replacing-a-source-file

But I get this error:
[VimeoApi] 401 Unauthorized request. Are you using a valid token?

Actually it makes sense, as it is quite weird that the documentation doesn't specify to add the authorization token in the header.

So, I added it, but then I still get another error:

[VimeoApi] https://api.vimeo.com/videos/307965347/versions - {"error":"Something strange occurred. Please try again.","link":null,"developer_message":"The body of this HTTP request is not formatted properly. Please check the content-type header and raw body.","error_code":2205}

… Matt Shoen,s: don't use the overloaded JSON symbol, and use a more explicit JSONNode class reference

JSONNode node = JSONNode.Parse(jsonString);
makes more sense than:
JSONNode node = JSON.Parse(jsonString);
…sage from the Vimeo server

- Added to VimeoRecorder Editor GUI
- Added a replaceExisting boolean field to VimeoRecorder

Went according to documentation in:
https://developer.vimeo.com/api/upload/videos#replacing-a-source-file

But I get this error:
[VimeoApi] 401 Unauthorized request. Are you using a valid token?

Actually it makes sense, as it is quite weird that the documentation doesn't specify to add the authorization token in the header.
So, I added it and I still get another error:
[VimeoApi] https://api.vimeo.com/videos/307965347/versions - {"error":"Something strange occurred. Please try again.","link":null,"developer_message":"The body of this HTTP request is not formatted properly. Please check the content-type header and raw body.","error_code":2205}
…sage from the Vimeo server

- Added to VimeoRecorder Editor GUI
- Added a replaceExisting boolean field to VimeoRecorder

Went according to documentation in:
https://developer.vimeo.com/api/upload/videos#replacing-a-source-file

But I get this error:
[VimeoApi] 401 Unauthorized request. Are you using a valid token?

Actually it makes sense, as it is quite weird that the documentation doesn't specify to add the authorization token in the header.
So, I added it and I still get another error:
[VimeoApi] https://api.vimeo.com/videos/307965347/versions - {"error":"Something strange occurred. Please try again.","link":null,"developer_message":"The body of this HTTP request is not formatted properly. Please check the content-type header and raw body.","error_code":2205}
@caseypugh
Copy link
Contributor

Finally got a chance to check out your PR. This looks pretty good so far! Your code still doesn't seem to pass the token in since it is setting the bearer token to false: https://github.com/vimeo/vimeo-unity-sdk/pull/66/files#diff-e36cc36407fd22c0311a67d7c8250c60R199

When I set it to true, I was able to replace my video. 🎉

It did uncover more bugs in the post-processing part though, but those seem like easy fixes. One fix is to update VimeoVideo to better validate null JSON responses.

// JSONNode video = { "duration": null }

if (video["duration"] != null) // this returns true (wrong)

if (video["duration"] != null && !video["duration"].IsNull) // this returns false (correct)

@caseypugh
Copy link
Contributor

Also, you should try passing in just the file name and not the full_path. The file_name is used just as a way to show a unique name for the new version you are uploading. The full path might be what is causing an issue...

@michaelboccara
Copy link
Contributor Author

Hey Casey

Thanks a lot for the tips. OK your remarks all make sense and I committed the changes, but I'm still getting an error. Actually I got several kinds of errors, every time it is different:

  • "The user is not allowed to perform that action."
  • "The requested video could not be found"
  • {"invalid_parameters":[{"field":"password","error_code":2223,"error":"Whoops! Please enter a password.","developer_message":"Password not provided for a password protected video."}],"error":"You have provided an invalid parameter. Please contact developer of this application.","link":null,"developer_message":"The parameters passed to this API endpoint did not pass Vimeo's validation. Please check the invalid_parameters list for more information.","error_code":2204}

In my test, I use the "Recorder" test scene, where I create a starfield video inside Project "Test". I set the privacy to "Only me". I named the video "Flying vees". That goes well, Then when I try to replace these are the errors I get, although I select the existing video correctly
Note that I we still need a fix in the editor UI to set the video name to be the same as the replaced one, at least for a default (rather than replace with "Unity Recording"). But the errors occur even when I make sure of setting the video name correctly...

The questions I have are:

  • Are we allowed to rename the video's title when doing a replacement ?
  • Is it OK to pass the video file's name as the "file_name" parameter of the Tus request ? I mean "Vimeo Unity Recording 724x470 2019.01.03 16.06.35.mp4"

@caseypugh
Copy link
Contributor

caseypugh commented Jan 3, 2019

I'm not sure how to help you if your result is inconsistent every time. Are you able to reproduce the same error state consistently? I do get this error every time:

{"error":"The requested video could not be found"}

But the video still gets replaced. Did you see if your video got replaced, despite the error showing up?

I think this error exists because it is trying to PATCH the wrong video ID. Seems like it has to do with this line https://github.com/vimeo/vimeo-unity-sdk/blob/master/Assets/Vimeo/Scripts/Config/VimeoVideo.cs#L76 - The regex is parsing out the version ID out of the URI rather than the video ID. e.g. For a replaced video, the URI looks like this: /videos/3093725/versions/1956041

To answer your Qs:

  • Yes, you should be able to rename the video, but I haven't tested.
  • Yes, that file name should work perfectly.

…bug was returning the version id instead of the video id.

Plus the null JSON node test needed a fix in more places.
@michaelboccara
Copy link
Contributor Author

OK you're right, well, almost. The video did get replaced, after I just got the "The requested video could not be found" error. But the next replacements will fail. Read till the end cause it seems your suggestion did solve the problem, although with some weird things on the way.

However. on the second time, I got the same error message and the video did not get replaced (it didn't open the browser window, and instead opened the Windows file explorer. Also the final message after the error was weird, with just this:
[VimeoPublisher] Uploaded video to
(no video id/url/name whatsoever

And, on the next try, I did get a different error:
"The user is not allowed to perform that action."
And no successful replacement.

Note that between the 1st and second try, I re-ran the game and I had to get another token from Vimeo to sign in (I requested the previous one while in game running mode so the Unity editor did not remember it). The first successful attempt was while staying in the same sign-in session (and even the same game session) between the first new recording and the second one with replacement.

And at the 3rd time, boom, after I requested a new token from Vimeo, I started again the game, recorded with replacing flag on, and I got the 3rd error about the wrong password.

Also, I looked and set a breakpoint at the line you mention, and I indeed see something weird, actually 2 things:
1- You're right that it should parse the video id not the version id
2- Actually the name field in the JSON response is empty. I would guess it should have been "V starfield".

So... I went ahead and made a fix for the regex issue (just removed the $ as the video id may not be at the end of the uri string). And.. it seems to fix the problem! Now I can replace the video again and again.

That's what I got so far. Yes the replacement happens the first time, in spite of the error message, but then in the 2nd replacement and beyond you start to get the weird errors. If you fix the regex bug it fixes all the error messages and the video seems to be replaced consistently.

Can we celebrate ?

@michaelboccara
Copy link
Contributor Author

And BTW, replacing the video name does work.
I added a few lines to set the video name field automatically when selecting a replaced video

@caseypugh
Copy link
Contributor

Hooray! Glad you were able to finally get it working

michaelboccara and others added 8 commits January 6, 2019 12:37
… to fetch updated videos list from vimeo at run-time, rather than relying on a refresh in the editor.

So I had to take a chunk of code from BaseEditor class and create a VideoFetcher run-time class, and use the factorized code in VimeoRecorder.
- BaseEditor should refer to the new VimeoFetcher rather than VimeoApi
- reorder inspector fields
# Conflicts:
#	Assets/Vimeo/Scripts/Config/PlayerSettings.cs
#	Assets/Vimeo/Scripts/Config/VimeoVideo.cs
#	Assets/Vimeo/Scripts/Editor/BaseEditor.cs
#	Assets/Vimeo/Scripts/Editor/VimeoRecorderEditor.cs
#	Assets/Vimeo/Scripts/Recorder/VimeoPublisher.cs
#	Assets/Vimeo/Scripts/Recorder/VimeoRecorder.cs
#	Assets/Vimeo/Scripts/Services/VimeoApi.cs
#	Assets/Vimeo/Scripts/Services/VimeoUploader.cs
@michaelboccara
Copy link
Contributor Author

Just merged master into this branch, to make life easier for vimeo repo owners. I specially tried to respect Vimeo SDK's coding style convention.

I thought I will give a summary here of what this feature provides:

  • At the Editor level, I added a toggle in the recorder named "replaceExisting". It will open another dropdown to select an existing video, and tell the recorder to overwrite this video

  • There is a mechanism with the video name that gets synced with the selected existing video. Selecting will update the video name, although you can modify the name afterwards while still overwriting. And on the opposite way, changing the name will also look up for an existing video in the list and change the selection if found. This allows the application to request an overwrite just by using the same name. You may decide not to integrate this mechanism, if you want the application to explicitly do this. Just look for the method SetVimeoIdFromName.

  • the 'vimeoVimeoId' member has been moved from PlayerSettings to VimeoSettings, as it is now relevant to both playing and recording/replacing

  • Some changes needed in the VimeoVideo class to fix the name parsing as needed for the selection replacement

  • A new class VideoFetcher, with code taken from BaseEditor. Its job is just to fetche for the list of existing projects and videos. This code was originally in BaseEditor as it was done only at the editor level, but now we need it at run time. BaseEditor now got a heavy diet as it just calls VideoFetcher. And do does VideoRecorder which now allows to overwrite videos at run-time based on the chosen name. Very useful for scripting (well at least for my script :).

  • And last but not least, VimeoApi has been updated with support for the video replacement URL, using the tus resumable upload protocol.

…ng quotas limit)

Signed-off-by: Michael Boccara <michael@smartertv.net>
…ideo.

2- Make sure to keep description unchanged.
3- Added capability to edit description when publishing videos.
4- Add support for paged request, for example when returning a list of more than 100 videos.
5- Update videos list after upload, in case we changed the video name or description
+ more callbacks in VimeoRecorder
@nsmith1024
Copy link

nsmith1024 commented Jul 17, 2019

So the player works properly with Unity WebGL now? Which one should i use this one , or the main one?

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

Successfully merging this pull request may close these issues.

4 participants