[FFmpeg-devel] support for reading / writing encrypted MP4 files

[Eran Kornblau]

> > +    case MKTAG('e','n','c','v'):        // encrypted video
> > +    case MKTAG('e','n','c','a'):        // encrypted audio
> > +        id = mov_codec_id(st, format);
> > +        st->codec->codec_id = id;
> 
> this seems missing a check for st->codec->codec_id being "unset"
> before setting it
> 
Will add it

> 
> > +        sc->format = format;
> > +        break;
> > +
> > +    default:
> > +        av_log(c->fc, AV_LOG_WARNING,
> > +               "ignoring 'frma' atom of '%.4s', stream format is '%.4s'\n",
> > +               (char*)&format, (char*)&sc->format);
> 
> the way these are printed would lead to different results on big and
> little endian
> 
Correct, however it's already done this way in a few other places in this file (search for %.4s)
Do you prefer that I will print it with %c%c%c%c as in ff_mov_read_stsd_entries ?

> > +
> > +    ret = av_aes_ctr_init(sc->cenc.aes_ctr, c->decryption_key);
> > +    if (ret) {
> > +        return ret;
> > +    }
> > +
> > +    return 0;
> 
> this could be simplfied
> also errors in ffmpeg are generally negative so
> if (ret < 0) seems more correct if a seperate check is left
> 
Ok, will change to 'return av_aes_ctr_init(...)'

> 
> > +    /* read the subsample count */
> > +    if (sc->cenc.auxiliary_info_pos + sizeof(uint16_t) > sc->cenc.auxiliary_info_end) {
> 
> the addition could overflow, which would be undefined behavior
> something like:
> sizeof(uint16_t) > sc->cenc.auxiliary_info_end - sc->cenc.auxiliary_info_pos
> is correcter
> 
Will change to what you suggested

> > +        if (input + clear_bytes + encrypted_bytes > input_end) {
> 
> the additions can overflow
> 
Will change to: 
if ((uint64_t)clear_bytes + encrypted_bytes > input_end - input) {
	

> [...]
> 
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
Thanks !

Eran