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

Adds buffer_size argument to parse #129

Conversation

tsitter-evertz
Copy link
Contributor

@tsitter-evertz tsitter-evertz commented Oct 19, 2023

We have an issue getting media info from TS files that we are stored in S3. The output is missing certain fields and/or tracks. i.e. Our tracks would not have a StreamOrder property, and sometimes the Menu track would not be returned. I believe it may be due to this open issue in MediaInfo.

As a workaround we found that increasing the buffer size in certain cases fixes the issue for the files we've been testing with, so hoping we can make this configurable.

Thanks!

@sbraz
Copy link
Owner

sbraz commented Oct 19, 2023

Hi and thanks for the PR. Would you have a small sample file that could be used to test the difference in behaviour when varying buffer sizes are used?
I'd also like to know if keeping this parameter makes sense in the long run. @JeromeMartinez, do you think there is a chance that the bug will be fixed and that this parameter will become useless in the future?

@JeromeMartinez
Copy link

there is a chance that the bug will be fixed

We have no ETA for the fix, struggling with lot of work on MediaInfo, but this issue has some kind of not so high but still high priority.

and that this parameter will become useless in the future

Fixing this issue does not make this option useless, some people use our direct API sometimes with smaller buffers e.g. 7*188 for 7 TS packets in a Ethernet frame (not the best example as it is about "file like" content, but this gives an idea of possibilities), or bigger buffers e.g. 1 MiB when they know they use only SSDs.

@JeromeMartinez
Copy link

Our tracks would not have a StreamOrder property, and sometimes the Menu track would not be returned.

@tsitter-evertz please open a ticket on https://github.com/MediaArea/MediaInfoLib/issues with a use case, I catch why there is a complete lack of detection of TS but not why StreamOrder property or menu track could be missing.

@tsitter-evertz
Copy link
Contributor Author

tsitter-evertz commented Oct 20, 2023

@JeromeMartinez will do, I have a file that I am able to replicate this issue but we're just trying to get permission from a client to share it, or else I'll have to generate a new file that I can share.

@juanitosvq
Copy link

juanitosvq commented Oct 20, 2023

Fixing this issue does not make this option useless, some people use our direct API sometimes with smaller buffers

@sbraz does this mean that we could get in the change proposed in this PR? We've temporarily vendored that parse method into our application to be able to customize the buffer size, it would be great if we could go back to using the library.

@sbraz
Copy link
Owner

sbraz commented Oct 20, 2023

@juanitosvq absolutely :) I was about to merge it but I noticed that the new parameter is not at the end of the list.
@tsitter-evertz, in the (very unlikely, I know) case that some consumers use parameters by position and not by name, could you please move it after output?

@tsitter-evertz
Copy link
Contributor Author

Good catch @sbraz, pushed that change.

@sbraz sbraz closed this in e6dc1cb Oct 24, 2023
@sbraz
Copy link
Owner

sbraz commented Oct 24, 2023

Thanks! I squashed your commits and committed this, I'll add Python 3.12 to the CI and create a new release soon.

@tsitter-evertz
Copy link
Contributor Author

Hi, do we have an ETA for the next release? Thanks

@tsitter-evertz
Copy link
Contributor Author

Our tracks would not have a StreamOrder property, and sometimes the Menu track would not be returned.

@tsitter-evertz please open a ticket on https://github.com/MediaArea/MediaInfoLib/issues with a use case, I catch why there is a complete lack of detection of TS but not why StreamOrder property or menu track could be missing.

Opened an issue here: MediaArea/MediaInfoLib#1899

@sbraz
Copy link
Owner

sbraz commented Oct 29, 2023

Hi, do we have an ETA for the next release? Thanks

I'm running the CI on this branch right now: https://github.com/sbraz/pymediainfo/tree/python312
Once it passes, I'll merge and tag as a new release.

@sbraz
Copy link
Owner

sbraz commented Oct 29, 2023

It's released as v6.1.0.

eyMarv pushed a commit to eyMarv/pymediainfo-pyroblack that referenced this pull request Mar 30, 2024
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.

4 participants