I agree . #4 makes the most sense. On Thu, Jan 5, 2017 at 12:38 PM, Eran Kornblau <eran.kornb...@kaltura.com> wrote:
> Hi all, > > We found today that this change: > https://github.com/FFmpeg/FFmpeg/commit/ca6cae73db207f17a0d5507609de12 > 842d8f0ca3 > can break the decryption of MP4s encrypted with common encryption. > > I would like to submit a patch for that, but wanted to consult first about > the best approach. > > The problem is that following this change the frames may not be traversed > sequentially from the beginning. > The issue we are currently experiencing is that the first audio frame gets > removed by mov_fix_index, > but the same issue will occur if there are frames that are duplicated or > dropped in the middle of the stream. > > The reason this breaks the decryption is that sc->cenc.auxiliary_info_pos > should be pointing to the auxiliary > info (=the encryption iv, in case of audio) of the next frame to be read. > It is initialized to the first frame in > mov_read_senc, incremented on every frame in cenc_filter, and updated in > mov_seek_auxiliary_info in case > of seek. > What happens here is that auxiliary_info_pos remains pointing to the first > frame in the track, but due to the > edit list, that is not the same frame as st->index_entries[0]. So the > frames get decrypted with wrong IVs, > and we get invalid data. > > I'm considering a few options for solving this: > > 1. The specific case we're currently experiencing (frames removed > from the beginning of the track) > can be easily solved by updating auxiliary_info_pos as part of > mov_fix_index. However, this solution > won't work for more complex edit lists (e.g. gaps in the middle). > > 2. Build a new auxiliary info buffer that will match the order of > the frames following the edit list. > For example, if some frames were removed from the beginning of the stream, > this new buffer > will be a suffix of the original buffer. If some frames were duplicated, > this new buffer will have > their auxiliary info duplicated as well. > > 3. Add a field to AVIndexEntry that will retain the original index > of the frame in the MP4 file. > So, for example, if a frame was removed from the beginning, > st->index_entries[0].index will > be 1 instead of 0. With this info, it will be possible to detect a frame > index discontinuity in > cenc_filter and recalculate the auxiliary info position accordingly. > > 4. Add a new array to MOVStreamContext that will retain the ranges > of frame indexes following > the edit list, e.g. if the edit list takes frames 30..60 and then > 100..150, the array will have 2 records > (30,60), (100, 150). This array will be built in mov_fix_index, and will > be used by cenc_filter. > > The pros/cons as I see it - > > 1. Pros: simple, cons: no support for complex edit lists > > 2. Cons: seems more complicated, and less efficient than the other > solutions > > 3. Pros: simple, cons: makes AVIndexEntry larger (and it seems care > has been taken to keep this > struct to the minimum...) > > 4. Pros: very low memory footprint, cons: slightly more complex than > #3 > > Personally, I'm leaning toward #4, but any comments are welcome > > Thank you > > Eran > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
