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

Enable media server remux #143

Merged
merged 26 commits into from
Jun 22, 2016
Merged

Enable media server remux #143

merged 26 commits into from
Jun 22, 2016

Conversation

enternoescape
Copy link
Contributor

No description provided.

…d not line up with with the offset indicated by the ByteBuffer object.
…knows to get the file size and perform SWITCH with the need to contact the the network encoder.
…fault channel was set to 1, the main program on a few channels coming in from the InfiniTV capture devices would never be detected.
…le null values and added a little more logging.
@@ -560,6 +594,10 @@ protected void sizeFile() throws java.io.IOException
availSize = xcoder.getVirtualTranscodeSize();
totalSize = xcoder.isTranscodeDone() ? availSize : getLargeFileSize(currFile.toString());
}
if (remuxer != null)
{
availSize = totalSize = remuxer.getFileSize();
Copy link
Member

Choose a reason for hiding this comment

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

You'll want to use getLargeFileSize(String) for the totalSize if the file is recording. This is used as an indicator in some of the playback clients to know that the file is currently recording (they do it by checking if totalSize > availSize); also be sure you handled the circular file case as that will be used for the preview window during playback (although your remuxer logic may already handle that case, I haven't got that far reviewing the code yet :) )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. There is a way for the network encoder to let the remuxer know that it needs to create a circular file.

@Narflex
Copy link
Member

Narflex commented Jun 21, 2016

And be sure to reply on here after you make more changes; it doesn't send me emails about it unless you do (so I would forget to look at it again otherwise).

…of 188. Explained why the ones that are multiples of 188 need to/should be. Changed size parameters for buffers that are multiples of 188 to show how the number was derived. Fixed file comparison to ensure that SWITCH doesn't transition to the exact same file.
@enternoescape
Copy link
Contributor Author

I think I have addressed the issues you pointed out. Other than the mistake I made comparing two files, none of them were significant enough to change anything in a noteworthy way, so I'm comfortable with this quick turnaround.


while (length - (offset + 377) > 0)
while (length - (offset + (188 * 2)) > 0)
Copy link
Member

Choose a reason for hiding this comment

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

I'm fairly sure this needs to be 377, not 376 for the value. Because if length is 377 here and offset is zero, then the conditional is true. Then offset gets incremented to 1. Then it will index array element 377 (which I'm assuming could correspond to the length of that array in certain cases).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're correct. The offset could be 1 over 188 * 2 and that would cause an out of bounds exception. Due to the alignment buffering, the code mentioned here doesn't get hit very often, so it was easy to miss. While I'm not a Java veteran, small mistakes like that are quite embarrassing.

@Narflex
Copy link
Member

Narflex commented Jun 22, 2016

OK, just two more comments from me. One I think is an actual bug; the other is just commenting. I am leaving for vacation tomorrow; so if you can get the changes in today I can merge them before I leave.

@enternoescape
Copy link
Contributor Author

I agree on the comments, it's always nice to see them when you're trying to figure out what the other guy was thinking. I always hate that when I'm looking at a project and comments are missing because the author thought the intent was obvious...and it's really not.

@Narflex Narflex merged commit e035fbb into google:master Jun 22, 2016
@Narflex
Copy link
Member

Narflex commented Jun 22, 2016

Yeah! New remuxer is merged. Thanks for all the hard work!

@enternoescape enternoescape deleted the enable-media-server-remux branch June 22, 2016 20:16
JREkiwi pushed a commit to JREkiwi/sagetv that referenced this pull request Dec 30, 2018
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