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

Media File Size field is too small #1771

Closed
seth-shaw-unlv opened this issue Mar 4, 2021 · 8 comments
Closed

Media File Size field is too small #1771

seth-shaw-unlv opened this issue Mar 4, 2021 · 8 comments

Comments

@seth-shaw-unlv
Copy link
Contributor

The File Size field we use on media uses a signed normal-size integer. According to Drupal this limits a file to being no more than 2,147,483,647 bytes, just under 2 GB, in size (and I think we can ignore the negative file sizes).

I discovered this when trying to ingest some video files that are just a bit larger than 2GB in size. The file entities were created just fine but the Media entities crashed.

We should, at least, update it to be unsigned (again, negative file sizes don't make any sense) which would allow ~4GB file sizes. Even that is still too small for some video files I've seen, not to mention disk images captured for born-digital collections, so it would be best to use "big integer" as well which (unsigned) gives us up to 16 exabytes.

While the fix for new sites is easy:

settings:
  unsigned: true
  size: big

we would need to create an update hook for existing sites to resize the relevant column.

@kayakr
Copy link
Contributor

kayakr commented Mar 4, 2021

@seth-shaw-unlv +1 for this. We ran into this issue back in the day https://kayakr.gitlab.io/presentation-2019-islandoracon-islandora-in-nz/#/22
2.2GB is only approx. 15min of HD video, so going way larger would be good. We have pending ingests for videos ~10GB or so.

@mjordan
Copy link
Contributor

mjordan commented Mar 5, 2021

Just thowing #1240 and #1577 in here as related issues..... 2 GB is not large enough for sure.

@adam-vessey
Copy link
Contributor

For an appropriate field widget and whatnot... should we be making use of something like the bigint module?

@seth-shaw-unlv
Copy link
Contributor Author

While we do need a big int, I think the one provided by Drupal (8 bytes), which gives us a field representing up to 16 exabytes, is certainly sufficient. I don't think we need to add an extra dependency for 11 more bytes on that column.

Of course, this doesn't stop anyone from using it themselves on their own site.

@adam-vessey
Copy link
Contributor

adam-vessey commented Mar 10, 2021

...? The bigint docs indicates 19 digits, not bytes... it's providing full support for the big int thing in fielded entities. Again, my thoughts are more with field widgets and whatnot... if you go to view a form which has a widget for plain integers, but try to force a big integer in, thinking you could get into weird validation errors.

@seth-shaw-unlv
Copy link
Contributor Author

Ah, I see what you mean... although their widget simply extends the core NumberWidget. The Formatter uses a string function to add thousands separators rather than PHP's number_format as Core does although it doesn't document why.

@adam-vessey
Copy link
Contributor

adam-vessey commented Mar 10, 2021

Actually... not sure about core's behaviour. number_format() acceptsfloat values, meaning it gets into the whole loss of precision... true, maybe less of an issue if it's just formatting things for display, and given the smaller numbers from plain integers it may not be encountered, but there do appear some issues relating to it/somewhat associated:

... If indeed, as per the second, the number of relevant decimal digits able to be represented in a float is platform-dependent... depending on the version PHP and architecture and whatnot... if 8 bytes gets one ~19 decimal digits, and they're suggesting one only gets 14 or 15 from a 64-bit(/8-byte) float (on common platforms?), one might see disparities show up for those larger numbers of between 14 and 19 digits, when formatted using number_format().

@mjordan
Copy link
Contributor

mjordan commented Mar 30, 2021

Resolved with Islandora/islandora#829.

@mjordan mjordan closed this as completed Mar 30, 2021
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

4 participants