search

JamesHeinrich / getID3

http://www.getid3.org/
Other
1.13k stars 245 forks source link

Fix for MPEG-1 video pixel aspect ratio #411

Closed bvibber closed 1 year ago

bvibber commented 1 year ago

The reciprocal of the actual value was being returned for MPEG-1 video. Files with square pixels were not affected, nor were MPEG-2 files listing a display aspect ratio.

Fixes https://github.com/JamesHeinrich/getID3/issues/410

bvibber commented 1 year ago

I don't understand why static analysis thinks the division can cause an error here -- the only possible error is divide by zero, which is guarded explicitly against.

StudioMaX commented 1 year ago

It might be dumb, but... We have a check isset($lookup[$mpeg_version][$rawaspectratio], which checks for the presence of a key in the array, but we need to look for values instead. UPD: forget this, I was wrong.

JamesHeinrich commented 1 year ago

... but we need to look for values instead. Can you explain what you mean further, please?

bvibber commented 1 year ago

There is a guard specifically for $ratio != 0 so I'm confused why 1 / $ratio is troublesome to the analyzer. :)

Krinkle commented 1 year ago

@brion The ratio is always casted to a float before this afaik, in which case 0.0 (as float rather than int) should work equally well, and satisfy the analyser. You could then also use strict equality, which would currently fail. I'm guessing the analyser is using strict equality to conservatively narrow down possible values.

bvibber commented 1 year ago

@Krinkle wrote:

@brion The ratio is always casted to a float before this afaik, in which case 0.0 (as float rather than int) should work equally well, and satisfy the analyser. You could then also use strict equality, which would currently fail. I'm guessing the analyser is using strict equality to conservatively narrow down possible values.

I'll try that in a followup and see if it helps. :)