On Sat, Feb 2, 2019 at 3:37 AM Michael Niedermayer <[email protected]> wrote:
> Is this change needed by some valid file ? > No, just a drive by fix since I happened to be looking at these types and the spec. > The change taken on its own is probably correct but its increasing the > range > of values without actually adding support for that thus quite possibly > introducing bugs. > > In case of the id, we use signed int for the entries this indexes into, > so the extended range is not going to work. also wonder if billions > of STSD entries in a stream will work when more than 1 cause problems > already. > > Another obvious issue is that currently we scan this table for negative > values and eliminate them but when this is made unsigned suddenly > larger values pass through. This has the potential to introduce > integer overflow bugs in later stages. More so unsigned overflows dont > show up in asan and these first/count values might affect array indexes > iam not saying theres a bug here just that its the set of circunstances > that is fertile for such bugs. > > variables related to indexes or counts always require extra scrutiny > when their type is changed > I really appreciate the scrutiny. Given I don't have a file that needs this, happy to abandon this patch. Chris _______________________________________________ ffmpeg-devel mailing list [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
