-
Notifications
You must be signed in to change notification settings - Fork 6k
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
TS Extractor: Fix problem with locating first PCR in HD-PVR recordings #7988
Conversation
TV shows recorded with Hauppauge HD-PVR will play, but do not allow seeking and do not show the length. This is because the TsDurationReader cannot locate the first PCR in the first 112800 bytes. Increasing that buffer to 122200 bytes solves the problem. To be on the safe side I am setting it to double the original, or 225600 bytes. The search for last PCR does not have this problem so I am leaving that at 122200 bytes.
Note: This is related to #5097. |
https://drive.google.com/file/d/1G1dOY5eu67kRu7h9IgNJglgqjsVgBB3M/view?usp=sharing Here's a sample file that this change does not help with. Can the scope of this fix be expanded to help with a broader range of mpegts files? Thanks in Advance! |
The current method to compute the duration is already requiring a non-trivial amount of memory so we should avoid increasing it too much. We should come up with a method that requires less memory in the future. @bennettpeter If increasing the buffer to 122200 bytes solves the issue, then I suggest we do that, to limit the memory consumption. I'll change the PR code myself. @troyhough With the current way of computing the duration, supporting the stream you provided would require increasing the buffer from 112 800 bytes to more than 488 800 bytes. I will therefore not make the change as it would significantly increase the memory consumption for all TS files. Hopefully your stream will be better supported once we have optimized the duration computation. |
@kim-vde, should we consider letting the app choose a value for this through a TsExtractor parameter? Seems like that'd be a bit more scalable than updating on a per-demand basis. I guess that if we were building a heuristic for this, that'd be based on the Transport Stream's bitrate, but that would require a considerably larger effort. |
@AquilesCanta I was thinking of fetching the data by chunks instead of having one large buffer. But the parameter is a good idea. I'll do that instead. |
@kim-vde A parameter would be good. I thought of that but it seems it would require changes in several classes to pass the parameter through, since the client code does not have access to the TSDurationReader class. |
@kim-vde Just so you know the test file I sent you was just a standard 30 minute HDHomerun recording from OTA, untouched. Probably the most common tuner on the market. Having said that I think it's pretty important to get seeking to work properly on something so common. Thanks for the consideration. |
@kim-vde Can I update the pull request to allow the specification of the buffer size to be set in DefaultExtractorsFactory and TsExtractor? I will do that if it is likely to be accepted. Are there any special standards to be followed in this? |
@bennettpeter I am actually busy doing that myself. It should be available in a few days. |
I see that the sample from @troyhough requires SEARCH_BYTES_START at 1200 packets and SEARCH_BYTES_END at 2600 packets, where my original pullrequest was using 1200 and 600. So there will need to be the ability to modify both of those, or use one modifiable value for both. |
Yes, I was going to use one variable for both. |
Context: Issue: #7988 PiperOrigin-RevId: 335608610
Context: Issue: #7988 PiperOrigin-RevId: 335608610
TV shows recorded with Hauppauge HD-PVR will play, but do not allow
seeking and do not show the length. This is because the TsDurationReader
cannot locate the first PCR in the first 112800 bytes. Increasing that
buffer to 122200 bytes solves the problem. To be on the safe side I am
setting it to double the original, or 225600 bytes. The search for last
PCR does not have this problem so I am leaving that at 122200 bytes.