search

WeTransfer / format_parser

file metadata parsing, done cheap
https://rubygems.org/gems/format_parser
Other
62 stars 18 forks source link

Parse dpx files correctly to avoid FloatDomainError #181

Closed nitika080289 closed 3 years ago

nitika080289 commented 4 years ago

For certain dpx format images, in the structure, horizontal_pixel_aspect and vertical_pixel_aspect are zero in the orientation. Since we calculate the pixel_aspect from these values, which in turn is used to determine the display_width_px and display_height_px, the value of pixel_aspect comes up to NaN. This later results in a FloatDomainError

As per my understanding, we should only find the display_width_px and display_height_px when the above values are non-zero, else default these to the width_px and height_px

Error NaN /app/vendor/bundle/ruby/2.7.0/gems/format_parser-0.22.1/lib/parsers/dpx_parser.rb:49 round /app/vendor/bundle/ruby/2.7.0/gems/format_parser-0.22.1/lib/parsers/dpx_parser.rb:49 call /app/vendor/bundle/ruby/2.7.0/gems/format_parser-0.22.1/lib/format_parser.rb:202 block in execute_parser_and_capture_expected_exceptions

nitika080289 commented 4 years ago

For creating a sample file with missing horizontal and vertical pixel aspect, I was finding it hard to modify an existing dpx file. Exiftool also does not support this. So, I used an online convertor to convert a sample jpeg file that we have in the fixtures to a dpx. This automatically set the horizontal and vertical aspects to 0, which we wanted to achieve. Please let me know if you feel that there can be a better approach to do this.

fabioperrella commented 4 years ago

For creating a sample file with missing horizontal and vertical pixel aspect, I was finding it hard to modify an existing dpx file. Exiftool also does not support this. So, I used an online convertor to convert a sample jpeg file that we have in the fixtures to a dpx. This automatically set the horizontal and vertical aspects to 0, which we wanted to achieve. Please let me know if you feel that there can be a better approach to do this.

Nice approach!!

The only problem is that this file is huge (7MB). Is it possible to create a smaller one?

nitika080289 commented 4 years ago

For creating a sample file with missing horizontal and vertical pixel aspect, I was finding it hard to modify an existing dpx file. Exiftool also does not support this. So, I used an online convertor to convert a sample jpeg file that we have in the fixtures to a dpx. This automatically set the horizontal and vertical aspects to 0, which we wanted to achieve. Please let me know if you feel that there can be a better approach to do this.

Nice approach!!

The only problem is that this file is huge (7MB). Is it possible to create a smaller one?

That can be done 👍

fabioperrella commented 4 years ago

@nitika080289 I noticed that you removed the file, but I think that something must be done to remove this file from git internals, otherwise it will increase the size of the repository.

I'm not sure if a push --force would do it, otherwise you can try something like this: https://stackoverflow.com/questions/3458685/how-can-i-completely-remove-a-file-from-a-git-repository

nitika080289 commented 3 years ago

@nitika080289 I noticed that you removed the file, but I think that something must be done to remove this file from git internals, otherwise it will increase the size of the repository.

I'm not sure if a push --force would do it, otherwise you can try something like this: https://stackoverflow.com/questions/3458685/how-can-i-completely-remove-a-file-from-a-git-repository

After multiple ways we tried, the file did not actually get deleted from the commits. We decided to leave it like that since it does not have a big impact.

martijnvermaat commented 3 years ago

If the file is not present in any commit that is reachable from a branch or tag anymore, then it will eventually be garbage-collected by GItHub.

nitika080289 commented 3 years ago

If the file is not present in any commit that is reachable from a branch or tag anymore, then it will eventually be garbage-collected by GItHub.

But I see that the file is still present in the first commit in which I added it 8dc54788062d6949a22e756779cf518b51d95e03

martijnvermaat commented 3 years ago

Ah right, then it will always be in the history if merging the complete branch. But if you choose "squash and merge" I'm pretty sure it will be gone. In that case your two commits will be squashed into one (that doesn't introduce the large file) and that is the only commit that will later be in the reachable commit graph.

nitika080289 commented 3 years ago

Ah right, then it will always be in the history if merging the complete branch. But if you choose "squash and merge" I'm pretty sure it will be gone. In that case your two commits will be squashed into one (that doesn't introduce the large file) and that is the only commit that will later be in the reachable commit graph.

This is nice. It would solve the problem then 👍