JamesHeinrich / getID3

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

Mpeg duration error due to error in GOP parsing #440

Closed hartman closed 3 months ago

hartman commented 3 months ago

https://phabricator.wikimedia.org/T360532

File: https://upload.wikimedia.org/wikipedia/commons/9/98/El_Presto_vs_Fabiola_Yañez.mpg For this file we were detecting an incorrect duration. Analysis shows that this is because there is an error in the GOP reading.

While reading group_of_pictures_header, the GOP counter is increased unconditionally, but the gop value is only added to $info['mpeg']['group_of_pictures'] depending on the bitrate field. In this file, multiple gop's seems to hit this particular exception.

Later on $last_GOP_id = max(array_keys($FramesByGOP)); is used as a key into that same array. So for the last gop, it finds gop counter id 516, but there are only 510 values in the gop array. This causes null access warnings on the console and the duration to be impossible to calculate.

I worked around this by setting an empty array value for these gop's

if (!empty($info['mpeg']['video']['bitrate_mode']) && ($info['mpeg']['video']['bitrate_mode'] == 'vbr')) {
...
} else {
    $info['mpeg']['group_of_pictures'][] = array();
    #echo "\nNo valid bitratemode on gop";
}

I have not investigated why there is suddenly no valid bitrate for some of these groups, but possible there's multiple streams or some other thing that the overall parser isn't accounting for.

JamesHeinrich commented 3 months ago

Thanks, I've replicated your change in https://github.com/JamesHeinrich/getID3/commit/143af3325ee40e77c5d041d3f674c8bdd4762dc7

hartman commented 3 months ago

I have confirmed btw. that these are mpeg sequences where the bitrate is suddenly not vbr. And it seems that these are the only sequence headers that have load_intra_quantiser_matrix or load_non_intra_quantiser_matrix set.

Not sure yet if this is an encoding error, or just a very uncommon valid situation.