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

fix #645 SeekableOutputStream #1350

Merged
merged 5 commits into from
Dec 14, 2019
Merged

fix #645 SeekableOutputStream #1350

merged 5 commits into from
Dec 14, 2019

Conversation

svorlauf
Copy link
Contributor

@svorlauf svorlauf commented Dec 9, 2019

Added a SeekCallback and SeekableOutputStream interface to write e.g. mp4 files into a byte array
As requested in issue #645

@saudet
Copy link
Member

saudet commented Dec 10, 2019

Thanks! Let's also add a small unit test for this to make sure it works.

@saudet
Copy link
Member

saudet commented Dec 10, 2019

Also, can we make this work with any ByteArrayOutputStream? It looks like we can do this by moving all logic out from SeekableOutputStream into SeekCallback, but am I missing something that prevents us from doing that?

@svorlauf
Copy link
Contributor Author

The problem ByteArrayOutputStream is that the byte array is not public but protected. So it either has to be extended or accessed by reflections. I also like to have an interface to maybe change the implementation without needing to add it to javacv first

@svorlauf
Copy link
Contributor Author

I am not sure about unit tests for video creation are there any examples? Or are you talking about the ´SeekableByteArrayOutputStream´ only?

@saudet
Copy link
Member

saudet commented Dec 12, 2019

The problem ByteArrayOutputStream is that the byte array is not public but protected. So it either has to be extended or accessed by reflections. I also like to have an interface to maybe change the implementation without needing to add it to javacv first

You're right, it's probably not worth hacking this too much. Let's put the interface outside as a top-level class as well. And to make it more consistent with conventions, let's name the method seek(...).

I am not sure about unit tests for video creation are there any examples? Or are you talking about the ´SeekableByteArrayOutputStream´ only?

Yes, just something to test SeekableByteArrayOutputStream and make sure it produces the expected array with a few typical calls.

@svorlauf
Copy link
Contributor Author

svorlauf commented Dec 12, 2019

Let's put the interface outside as a top-level class as well. And to make it more consistent with conventions, let's name the method seek(...).

done

I added some test cases for write and seek. There is also added on test to compare .mp4 written to file and SeekableOutputStream but i do not know wether it might fail depending on the environment if the encoder is not deterministic. If so we will have to remove the test

@svorlauf
Copy link
Contributor Author

I tried to follow code conventions. If there is anything amiss just give some hints (e.g. missing documentation)

@svorlauf
Copy link
Contributor Author

By the way. I am new to GitHub. Ist there besides the issues and pull requests any forum for general questions? E.g. code style, Java compatibility version etc

@saudet
Copy link
Member

saudet commented Dec 13, 2019

I tried to follow code conventions. If there is anything amiss just give some hints (e.g. missing documentation)

I try to follow the conventions from the original document here:
https://www.oracle.com/technetwork/java/codeconvtoc-136057.html
And when there isn't any contradictions, the ones from here as well:
https://google.github.io/styleguide/javaguide.html
But there is a lot that isn't documented either, so I just go with what makes most sense and what I see others do. Rereading those documents again though, there is a lot I'm not doing like they are doing, so maybe I'll change a few things... (There's also a lot of Javadoc missing. I could use some help with that, for example, see issue #1356.)

By the way. I am new to GitHub. Ist there besides the issues and pull requests any forum for general questions? E.g. code style, Java compatibility version etc

As mentioned in the README.md file, there is a mailing list here:

But because accessing Google from China and a few other countries isn't reliable, many people just post their questions as GitHub issues. There isn't anything else than that.

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.

2 participants