-
Notifications
You must be signed in to change notification settings - Fork 287
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 FFmpeg support to open video files #30
Conversation
7c8fc9c
to
66df035
Compare
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
CLAs look good, thanks! |
# License for the specific language governing permissions and limitations under | ||
# the License. | ||
# ============================================================================== | ||
"""Tests for SequenceFileDataset.""" |
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.
Update this to VideoDataset
instead of SequenceFileDataset
? Same for other places
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.
Thanks @terrytangyuan, the PR has been updated.
272d247
to
abadf38
Compare
I think this PR is almost ready. I am working on adding Ubuntu 16.04 and 14.04 support (on top of 18.04 support that is already in place). Will update the PR shortly to completion. |
a6c0ae3
to
a928008
Compare
I think this PR is ready. We support Ubuntu 14.04, 16.04, and 18.04 at the moment, with one distributed package in pip. Tests also pass on those platforms. |
This is a thing of beauty. I especially appreciate the multi-version tooling. Since these are LGPL, we have to make sure to only release dynamically linked versions. Can we make sure (with a test/scripting) that this is always going ot be the case? |
@martinwicke The library only uses header files from ffmpeg to compile and build. In Bazel there is a check-license option though I don't know how reliable it is. I think some additional tooling might be possible (maybe a module in bazel) to enforce only header files are included for LGPL types of libraries . I will take a look at the license tooling issue. |
I think as long as we make sure all releases are built with dynamic linking, we should be ok. |
This fix adds a preliminary video file support (`VideoDataset`) by linking and using FFmpeg's libraries. The `video.VideoDataset` takes input of filename list, and generates outputs of RGB24 images (`uint8`) in `(height, width, 3)`. The reason to link against FFmpeg's libraries is because FFmpeg has a LGPL 2.1+ license. On Ubuntu 18.04, FFmpeg libraries consists of: ``` libavformat.so.57 libavcodec.so.57 libavutil.so.55 libswscale.so.4 ``` Because FFmpeg only guarantee's major version conpatibility, linking to a lower version (e.g., libavformat.so.54 on Ubuntu 14.04) may not work. To support multiple versions of Ubuntu, multiple copies of header file combined with linking to different versions of .so might be needed. It needs to be fully tested though and only limited major version should be supported. At the moment, only ffmpeg libraries in Ubuntu 18.04 are used. To install required libraries on Ubuntu 18.04: ``` apt-get -y -qq install libavformat57 libavcodec57 libavutil55 libswscale4 ``` To build the pip package: ``` bazel build -s --verbose_failures build_pip_pkg ``` Signed-off-by: Yong Tang <[email protected]>
Signed-off-by: Yong Tang <[email protected]>
Signed-off-by: Yong Tang <[email protected]>
Signed-off-by: Yong Tang <[email protected]>
Signed-off-by: Yong Tang <[email protected]>
Signed-off-by: Yong Tang <[email protected]>
Signed-off-by: Yong Tang <[email protected]>
So that build could always succeed. In runtime system .so files will be used. Signed-off-by: Yong Tang <[email protected]>
…ave Ubuntu 18.04 support directly. Signed-off-by: Yong Tang <[email protected]>
Encountered quite some issues with ``` Use --sandbox_debug to see verbose messages from the sandbox /usr/bin/ld.gold: error: cannot find libavformat.so.57 /usr/bin/ld.gold: error: cannot find libavcodec.so.57 /usr/bin/ld.gold: error: cannot find libavutil.so.55 /usr/bin/ld.gold: error: cannot find libswscale.so.4 collect2: error: ld returned 1 exit status Target //tensorflow_io/video:video_py_test failed to build ``` Believe it is related to (or similiar): https://stackoverflow.com/questions/52386530/linker-fails-in-sandbox-when-running-through-bazel-but-works-when-sandboxed-comm?rq=1 Signed-off-by: Yong Tang <[email protected]>
(By default Travis CI uses Ruby) Signed-off-by: Yong Tang <[email protected]>
…ainer Signed-off-by: Yong Tang <[email protected]>
Signed-off-by: Yong Tang <[email protected]>
…nore error Signed-off-by: Yong Tang <[email protected]>
Signed-off-by: Yong Tang <[email protected]>
Signed-off-by: Yong Tang <[email protected]>
Signed-off-by: Yong Tang <[email protected]>
…brary and use `cc_import_library` rule to prevent including GPL code. Signed-off-by: Yong Tang <[email protected]>
Signed-off-by: Yong Tang <[email protected]>
@terrytangyuan @martinwicke The PR has been updated. I added a customized implementation of bazel rule def ( It is pretty much similar to I think this FFmpeg PR is ready, please take a look and let me know if there are any issues. |
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.
LGTM!
Thanks. Will merge this PR in, and have a follow up PR with an example. |
* feat: reading from bigtable (#2) Implements reading from bigtable in a synchronous manner. * feat: RowRange and RowSet API. * feat: parallel read (#4) In this pr we make the read methods accept a row_set reading only rows specified by the user. We also add a parallel read, that leverages the sample_row_keys method to split work among workers. * feat: version filters (#6) This PR adds support for Bigtable version filters. * feat: support for other data types (#5) * fix: linter fixes (#8) * feat docs (#9) * fix: building on windows (#12) * fix: refactor bigtable package to api folder (#14) moved bigtable to tfensorflow_io.python.api * fix: tests hanging (#30) changed path to bigtable emulator and cbt in tests moved arguments' initializations to the body of the function in bigtable_ops.py fixed interleaveFromRange of column filters when using only one column * fix: temporarily disable macos tests (#32) * disable tests on macos Co-authored-by: Kajetan Boroszko <[email protected]> Co-authored-by: Kajetan Boroszko <[email protected]>
This fix adds a preliminary video file support (
VideoDataset
) by linking and using FFmpeg's libraries.The
video.VideoDataset
takes input of filename list, and generates outputs of RGB24 images (uint8
) in(height, width, 3)
.The reason to link against FFmpeg's libraries is because FFmpeg has a LGPL 2.1+ license.
On Ubuntu 18.04, FFmpeg libraries consists of:
Because FFmpeg only guarantee's major version conpatibility, linking to a lower version (e.g., libavformat.so.54 on Ubuntu 14.04) may not work.
To support multiple versions of Ubuntu, multiple copies of header file combined with linking to different versions of .so might be needed.
It needs to be fully tested though and only limited major version should be supported.
At the moment, only ffmpeg libraries in Ubuntu 18.04 are used.UPDATE:
At the moment, Ubuntu 14.04, 16.04, and 18.04 are all supported. We build 3 .so files and distributed at the same time:
Additional platform could be added if there is enough need from the community.
To install required libraries on Ubuntu 18.04:
To build the pip package:
NOTE: This PR fixed #11
Signed-off-by: Yong Tang [email protected]