2018-01-08 23:16 GMT+01:00 Jacob Trimble <modmaker-at-google....@ffmpeg.org>: >> You can't remove API just like that without a deprecation period. >> Add a new av_aes_ctr_set_full_iv() function, and either leave >> av_aes_ctr_set_iv() alone or deprecate it if it effectively becomes >> superfluous after adding the new function. >> >> Also, this patch needs to be split in three. One for the aes_crt >> changes, one for the encryption_info changes, and one for mov changes, >> with a minor libavutil version bump for the former two, and the >> corresponding APIChanges entry. >> Alternatively, you could also squash the new encryption_info API from >> this patch into the one that actually introduces the entire feature. > > Whoops, I thought that was internal-only. Done and split into its own change. > > On Sat, Jan 6, 2018 at 7:30 AM, Carl Eugen Hoyos <ceffm...@gmail.com> wrote: >> 2018-01-05 20:49 GMT+01:00 Jacob Trimble <modmaker-at-google....@ffmpeg.org>: >> >>> + if (!frag_stream_info->encryption_index) { >>> + frag_stream_info->encryption_index = >>> av_mallocz(sizeof(MOVEncryptionIndex)); >> >> sizeof(variable), please.
Do you disagree? I have no strong opinion, but I thought it makes the code more robust... >> [...] >> >>> + sample_count = avio_rb32(pb); >>> + >>> + encryption_index->encrypted_samples = >>> av_mallocz_array(sizeof(AVEncryptionInfo*), sample_count); >> >> This should be avoided if possible, see below. >> >>> + if (!encryption_index->encrypted_samples) { >>> return AVERROR(ENOMEM); >>> } >>> + encryption_index->nb_encrypted_samples = sample_count; >>> >>> - return av_aes_ctr_init(sc->cenc.aes_ctr, c->decryption_key); >>> + for (i = 0; i < sample_count; i++) { >> >> Please check here for eof... >> >>> + ret = mov_read_sample_encryption_info(c, pb, sc, >>> &encryption_index->encrypted_samples[i], use_subsamples); >> >> ... and insert a realloc here to avoid the large allocation above, see >> 1112ba01. > > Done. > + if (use_subsamples) { > + subsample_count = avio_rb16(pb); > + for (i = 0; i < subsample_count && !pb->eof_reached; i++) { > + unsigned int min_subsamples = FFMIN(FFMAX(i, 1024 * 1024), > subsample_count); > + subsamples = av_fast_realloc((*sample)->subsamples, &alloc_size, > + min_subsamples * > sizeof(*subsamples)); The reason I did not comment on this block in the last mail is that iiuc, the maximum allocation here is INT16_MAX * 8 which seems acceptable (and there cannot be a realloc with the chosen initial limit). > + sample_count = avio_rb32(pb); You have to check here... + for (i = 0; i < sample_count && !pb->eof_reached; i++) { + unsigned int min_samples = FFMIN(FFMAX(i, 1024 * 1024), sample_count); + encrypted_samples = av_fast_realloc(encryption_index->encrypted_samples, &alloc_size, + min_samples * sizeof(*encrypted_samples)); ... if this product could overflow for the final reallocation, see 1112ba01. Thank you, Carl Eugen _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel