Hi, 2016-05-20 15:09 GMT+02:00 foo86 <fooba...@gmail.com>:
>> Not that the patch is not ok, but I have a few uneducated questions: >> 1) Given the get_bits_long(gb, k) afterwards, won't that code cause >> overreads for corrupted bitstreams? > > This will cause overread, but that should not be a problem for checked > bitstream reader. > >> 2) I haven't checked the calling code, but consequently, wouldn't it >> be better to first check that at least k+1 bits are available? > > I don't think this is necessarry. Fixed length overread is safe; adding > explicit check will make code less readable IMO (and possibly slower). I think the checked bitstream reader takes care of these. >> 3) 128 is already fairly large; is the new code for valid bitstreams >> (in the sense of specs and actually generated) or for corrupted >> bitstreams? I don't know where the parsing is validated afterwards >> (e.g. if there have been overreads or invalid values parsed) > > This is for valid bitstreams. There is no indication of limit on maximum > Rice code length in the XLL spec, hence existing constant is not > strictly "valid" (but it always worked in practice with existing > encoders). Reference decoder also loops indefinitely until it sees stop > bit here. OK. Out of curiosity, what are classical values there? "common" and max (128?). > Parsing is validated at the end of chs_parse_band_data(), there is an > explicit check whether overread has occured (and if it has, entire > segment is zeroed out). OK, so I think this patch is completely fine. Thanks, -- Christophe _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
