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

Legal(?) and size issues with bundled test clips #3009

Closed
mstorsjo opened this issue Aug 6, 2018 · 8 comments
Closed

Legal(?) and size issues with bundled test clips #3009

mstorsjo opened this issue Aug 6, 2018 · 8 comments

Comments

@mstorsjo
Copy link
Contributor

mstorsjo commented Aug 6, 2018

Ping @ethanhugg

The recently added support for b-frames also came with a few test samples, which turn out to be clips from actual movie trailers. Is this ok copyright wise for this project?

And even if they'd be ok license wise, these clips are way way too big for the testsuite. When running the whole testsuite in valgrind (in order to catch use of uninitialized data, reads/writes out of bounds etc, including in assembly which address sanitizer doesn't catch), these 4 test clips more than doubled the runtime of the testsuite, from around 30 minutes before, to 70 minutes now, with these 4 clips taking over 40 minutes to decode when run in valgrind.

Either the clips must be reduced, or I have to reconsider running regular testing of this project in valgrind.

@fluffy
Copy link
Member

fluffy commented Aug 6, 2018

I think we would prefer to use test clips which were well accepted test data that was commonly used in to develop video codecs. That type of data has copyright that clearly allows us to use it and is also well known what it should look like to video experts so it tends to be easier to catch mistakes in. How long are these clips right now? I suspect that there is no real gain to longer clips and to be able to speed up testing, we should probably use clips that are long enough to test the system but as short as possible.

@mstorsjo
Copy link
Contributor Author

mstorsjo commented Aug 6, 2018

The current tests for high profile are 200, 300, 300 and 580 frames long, but they are in 720p and 1080p, and weigh in at about 40 MB - imo total overkill for this test case. (And then the tests don't even cover CAVLC-encoded B-slices, which the pull request didn't contain support for.)

@sijchen
Copy link
Contributor

sijchen commented Aug 7, 2018

@xiaotianshi2 we will need to remove the test-clips for B-frames if the copy-right problem is not clear. Thanks.

@xiaotiansf
Copy link
Contributor

@sijchen If that's the issue, it had better to remove the trailer test cases.

@GuangweiWang
Copy link
Collaborator

@xiaotiansf
I have removed two test samples on the PR#3010
can you help to check the other two files?

@xiaotiansf
Copy link
Contributor

@GuangweiWang

I am sure these two files have no licence issue.
res/HighProfile_B_Frame_1920x1080p_2397fps.h264
res/HighProfile_B_Frame_1920x1080p_30fps.h264

@mstorsjo
Copy link
Contributor Author

mstorsjo commented Aug 7, 2018

@xiaotiansf License issues aside, can you produce samples with similar code coverage but shorter clips and much lower resolution? It doesn't need to be HD content just to test decoding of b-frames.

@xiaotiansf
Copy link
Contributor

@mstorsjo
Sure and I'll find and add some suitable samples.

@ruil2 ruil2 closed this as completed Sep 13, 2018
mstorsjo added a commit to mstorsjo/openh264 that referenced this issue Sep 13, 2018
…x320_CABAC_Bframe_9.264

This solves part of issue cisco#3009 by using more reasonably sized
test samples.
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

No branches or pull requests

6 participants