-
Notifications
You must be signed in to change notification settings - Fork 694
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
Fix webm embedding (#468) #474
Conversation
Obviously, you cannot use links to the source folder. @JafarAbdi, do you have an idea how to fix the broken video link? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this approach should work on the online page, too -- the previous approach was generating an img
tag rather than video
tag
Previously generated HTML code
<a class="reference internal image-reference" href="../../_images/rviz_joints_nullspace.webm"><img alt="../../_images/rviz_joints_nullspace.webm" src="../../_images/rviz_joints_nullspace.webm" style="width: 700px;"></a>
Currently generated HTML code
<video controls="" autoplay="" name="media"><source src="file:///home/jafar/workspaces/web/moveit_tutorials/doc/quickstart_in_rviz/rviz_joints_nullspace.webm" type="video/webm"></video>
How can an absolute file path like |
.. raw:: html | ||
|
||
<div style="position: relative; padding-bottom: 5%; height: 0; overflow: hidden; max-width: 100%; height: auto;"> | ||
<iframe width="700px" height="400px" src="../../../../doc/quickstart_in_rviz/rviz_joints_nullspace.webm" frameborder="0" allow="autoplay; encrypted-media" allowfullscreen></iframe> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to @rhaschke's comment. A relative path seems brittle. There has to be a better way to do this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mlautman: Why do you then approve?
The relative path here is transformed into an absolute path by sphinx, which will be broken then on the web.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the way to go is to use the video plugin for sphinx that @felixvd mentioned.
@JafarAbdi, can you have a look at this, please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rhaschke I approved because I didn't have a better suggestion and it seemed to fix the problem. Also, there are a lot of other relative paths in the tutorials, albeit none as aggressive as this, so I didn't think that my reservations were disqualifying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While other relative paths stay relative, this one here becomes absolute and will - for this reason - break. Ping @JafarAbdi to have a look at the proposed video plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to build the doc on my fork, but Github pages gives an error. I suppose there's more to it. Is there another way to check that this works, or fix this @JafarAbdi ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running the following line should be sufficient https://github.com/ros-planning/moveit_tutorials/blob/master/.travis.yml#L47-L52
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried executing the lines from this version of that file, but neither the moveit/moveit:master-source
nor the moveit/moveit:master-ci
docker image have sphinx-build
or gem
installed. Even after running apt-get install gem
, the newer htmlproofer.sh
script gives line 16: gem: command not found
. What am I missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, I remember having trouble setting up the website locally but I don't how I got it working later, this PR fix the issue by converting it to GIF
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I commented over there. Figuring out how to embed the webm would really be better than encoding video in a 30+ year old, barely compressed format.
Closing and re-opening to trigger Travis. |
I propose we close this in favor of #550 |
This goes towards fixing the formatting issues that caused the webm not to display in #468. It runs with
build_locally.sh
, but I am not sure that it will work online, since the webm file is not copied to the build directory like the images are. I am not sure if Github Pages allows using the direct link to the repo.If this approach seems off, I'll add a png and a link to the webm. It would be prettier if we had videos, though.
I also found this sphinx plugin for videos.
Checklist