Skip to content

Support for video embedding#550

Merged
davetcoleman merged 4 commits intomoveit:masterfrom
DLu:webm
Nov 30, 2020
Merged

Support for video embedding#550
davetcoleman merged 4 commits intomoveit:masterfrom
DLu:webm

Conversation

@DLu
Copy link
Copy Markdown
Contributor

@DLu DLu commented Oct 26, 2020

Description

Based on feedback in #546, I spun the video embedding out into this PR, which

  • Copies the source code from sphinx-contrib/video directly. I was unable to figure out the proper way to install a dependency on the upstream extension.
  • Moves the video file in question to the static folder, because the above extension doesn't seem to copy it to the destination folder like the image extension does.
  • Updates the embedding by directly embedding the HTML required for the video to play

~Its not the most elegant solution, but it works. I welcome feedback on how to make it more elegant. ~

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • While waiting for someone to review your request, please consider reviewing another open pull request to support the maintainers

@DLu DLu requested a review from felixvd October 26, 2020 13:01
@felixvd
Copy link
Copy Markdown
Contributor

felixvd commented Oct 26, 2020

Thanks a lot for putting in the effort! I love that the video seems to work, and I don't mind the file living in static necessarily, but copied code needs to be maintained, so I'm wary about that.

I'm not familiar with the build process, so I'm not sure what you mean by install a dependency on the upstream extension. Wouldn't it work if you add the install instructions for this extension to this script?

@DLu
Copy link
Copy Markdown
Contributor Author

DLu commented Oct 26, 2020

I am just learning my way around sphinx. There seems like there should be a way to link to the released extension simply by adding sphinxcontrib.video in the conf.py, but that requires that the extension is on the path somewhere and I'm unclear what the sphinx way to do that is. Add a git clone to the html_proofer script?

@felixvd
Copy link
Copy Markdown
Contributor

felixvd commented Oct 26, 2020

I don't know myself, but you can probably try what works in a master-source docker container.

@AndyZe
Copy link
Copy Markdown
Member

AndyZe commented Oct 27, 2020

I'll just mention, one of the tutorials already has an embedded Youtube video.

https://ros-planning.github.io/moveit_tutorials/doc/realtime_servo/realtime_servo_tutorial.html

Comment thread _scripts/video.py Outdated
Comment thread doc/quickstart_in_rviz/quickstart_in_rviz_tutorial.rst Outdated
@DLu
Copy link
Copy Markdown
Contributor Author

DLu commented Nov 12, 2020

I've updated this so that the video loops forever like a gif would.

When I built it locally using the ./build_locally.sh script, the generated html is

<p>You can use the <strong>Joints</strong> tab to move single joints and the redundant joints of 7-DOF robots. Try moving the “null space exploration” slider as shown in the animation below.</p>
<video width="700px" nocontrols="true" autoplay="true" loop="true">
<source src="../../_static/rviz_joints_nullspace.webm" type="video/webm">
Illustration
</video>

Copy link
Copy Markdown
Contributor

@felixvd felixvd left a comment

Choose a reason for hiding this comment

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

The directory/location might not technically be the cleanest way to write it, but if it works I'm happy with it.

edit: I'm aware that I wrote the location first :) Just saying.

@DLu
Copy link
Copy Markdown
Contributor Author

DLu commented Nov 20, 2020

I believe CI would pass for this now that #556 is merged.

@rhaschke
Copy link
Copy Markdown
Contributor

Closing and reopening to trigger CI.

@rhaschke rhaschke closed this Nov 21, 2020
@rhaschke rhaschke reopened this Nov 21, 2020
@DLu DLu closed this Nov 25, 2020
@DLu DLu reopened this Nov 25, 2020
@DLu DLu requested a review from davetcoleman November 30, 2020 15:27
Copy link
Copy Markdown
Member

@davetcoleman davetcoleman left a comment

Choose a reason for hiding this comment

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

Seems like this has been greatly simplified, for better or worse :-)

@davetcoleman davetcoleman merged commit 5d9c8ed into moveit:master Nov 30, 2020
@davetcoleman
Copy link
Copy Markdown
Member

@DLu this is probably worth adding to the repo README on best practice for embedding video/gif, so we don't go through this again

@DLu DLu deleted the webm branch December 1, 2020 02:26
@felixvd felixvd mentioned this pull request Feb 10, 2021
Abishalini pushed a commit to Abishalini/moveit_tutorials that referenced this pull request Apr 29, 2021
* Support for video embedding

* Remove video dependency in favor of raw html

* Loop=true

* Update default text
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.

5 participants