[FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data
nfxjfg at googlemail.com
Wed Dec 20 22:23:55 EET 2017
On Wed, 20 Dec 2017 12:07:09 -0800
Jacob Trimble <modmaker-at-google.com at ffmpeg.org> wrote:
> On Tue, Dec 19, 2017 at 3:05 PM, wm4 <nfxjfg at googlemail.com> wrote:
> > On Tue, 19 Dec 2017 14:20:38 -0800
> > Jacob Trimble <modmaker-at-google.com at ffmpeg.org> wrote:
> >> > I don't think this is sane. So far, side data could simply be copied
> >> > with memcpy, and having pointers to non-static data in side data would
> >> > break this completely
> >> Then how about storing the data like it is now (the encryption info
> >> followed by the subsample array), but not have a pointer? Then there
> >> will be several helper methods to get the subsample array from the
> >> side-data. For example,
> >> av_encryption_info_get_subsamples(AVPacketEncryptionInfo*) or
> >> av_encryption_info_get_subsamples(AVPacket*) (since there will only be
> >> one instance of it).
> > I guess that would work? Not particularly fond of the idea anyway. I
> > think the functions would probably work on the side data byte array,
> > maybe.
> I'm not fond of this either, but I can't think of a way to allow a
> dynamic number of elements while supporting memcpy and not requiring
> the app to "parse" the side-data.
> So here is may latest attempt. This has a binary format inside the
> side-data that allows memcpy to work, but there is a public struct
> that apps will interact with. There are two methods used to convert
> between the two so the app doesn't have to. Even though this is a
> binary format, it is not actually a wire format since the subsamples
> are stored as-is, so they are host byte ordered. Also, as Michael
> requested, this uses dynamic sized key ID and IV.
I think your new patch idea is pretty sane. I'd explicitly document
that the encryption side data format is not part of the ABI, and that
applications must access it with the av_encryption_info_*() functions.
Also I think it would be better to avoid the memory allocation
messiness by copying all data, instead of implicitly referencing the
AVPacket data. Also provide a free function. (That is assuming you're
fine with a copy - creating AVPacket references copies all side data
anyway, so the whole code base assumes side data copies are cheap.)
If AVEncryptionInfo ever needs to be extended, it would also be better
to just let av_encryption_info_get() return a newly allocated struct,
and to exclude the size of the struct from the ABI. Then adding new
fields would not require waiting for an ABI bump.
More information about the ffmpeg-devel