[FFmpeg-devel] libavutil/encryption_info: Allow multiple init info.
Michael Niedermayer
michael at niedermayer.cc
Sat May 26 04:13:13 EEST 2018
On Fri, May 25, 2018 at 02:16:47PM -0700, Jacob Trimble wrote:
> On Mon, May 21, 2018 at 9:25 AM, Jacob Trimble <modmaker at google.com> wrote:
> > On Mon, May 14, 2018 at 4:49 PM, Jacob Trimble <modmaker at google.com> wrote:
> >> On Tue, May 8, 2018 at 3:47 PM, Michael Niedermayer
> >> <michael at niedermayer.cc> wrote:
> >>> On Mon, May 07, 2018 at 04:59:33PM -0700, Jacob Trimble wrote:
> >>>> On Mon, May 7, 2018 at 3:18 PM, Michael Niedermayer
> >>>> <michael at niedermayer.cc> wrote:
> >>>> > On Mon, Apr 23, 2018 at 11:03:57AM -0700, Jacob Trimble wrote:
> >>>> >> While integrating my encryption info changes, I noticed a problem with
> >>>> >> the init info structs. I implemented them as side-data on the Stream.
> >>>> >> But this means there can only be one per stream. However, there can
> >>>> >> be multiple 'pssh' atoms in a single stream (e.g. for key rotation or
> >>>> >> multiple key systems). (sorry for not noticing sooner)
> >>>> >>
> >>>> >> Attached is a patch to fix this by making the init info a
> >>>> >> singly-linked-list. It is ABI compatible and is still easy to use and
> >>>> >> understand. The alternative would be to break ABI and have the
> >>>> >> side-data methods return an array of pointers. I could do that
> >>>> >> instead if that is preferable.
> >>>> >
> >>>> >> encryption_info.c | 154 +++++++++++++++++++++++++++++++++++-------------------
> >>>> >> encryption_info.h | 5 +
> >>>> >> 2 files changed, 106 insertions(+), 53 deletions(-)
> >>>> >> e5eecc73a6997bbd11541771372d38ea9c3972a7 0001-libavutil-encryption_info-Allow-multiple-init-info.patch
> >>>> >> From bb941a77e882e93629d63d63059d0063b9519e29 Mon Sep 17 00:00:00 2001
> >>>> >> From: Jacob Trimble <modmaker at google.com>
> >>>> >> Date: Mon, 23 Apr 2018 10:33:58 -0700
> >>>> >> Subject: [PATCH] libavutil/encryption_info: Allow multiple init info.
> >>>> >>
> >>>> >> It is possible for there to be multiple encryption init info structure.
> >>>> >> For example, to support multiple key systems or in key rotation. This
> >>>> >> changes the AVEncryptionInitInfo struct to be a linked list so there
> >>>> >> can be multiple structs without breaking ABI.
> >>>> >>
> >>>> >> Signed-off-by: Jacob Trimble <modmaker at google.com>
> >>>> >> ---
> >>>> >> libavutil/encryption_info.c | 154 +++++++++++++++++++++++-------------
> >>>> >> libavutil/encryption_info.h | 5 ++
> >>>> >> 2 files changed, 106 insertions(+), 53 deletions(-)
> >>>> >>
> >>>> >> diff --git a/libavutil/encryption_info.c b/libavutil/encryption_info.c
> >>>> >> index 20a752d6b4..9935c10d74 100644
> >>>> >> --- a/libavutil/encryption_info.c
> >>>> >> +++ b/libavutil/encryption_info.c
> >>>> >> @@ -160,13 +160,16 @@ uint8_t *av_encryption_info_add_side_data(const AVEncryptionInfo *info, size_t *
> >>>> >> }
> >>>> >>
> >>>> >> // The format of the AVEncryptionInitInfo side data:
> >>>> >> -// u32be system_id_size
> >>>> >> -// u32be num_key_ids
> >>>> >> -// u32be key_id_size
> >>>> >> -// u32be data_size
> >>>> >> -// u8[system_id_size] system_id
> >>>> >> -// u8[key_id_size][num_key_id] key_ids
> >>>> >> -// u8[data_size] data
> >>>> >> +// u32be init_info_count
> >>>> >> +// {
> >>>> >> +// u32be system_id_size
> >>>> >> +// u32be num_key_ids
> >>>> >> +// u32be key_id_size
> >>>> >> +// u32be data_size
> >>>> >> +// u8[system_id_size] system_id
> >>>> >> +// u8[key_id_size][num_key_id] key_ids
> >>>> >> +// u8[data_size] data
> >>>> >> +// }[init_info_count]
> >>>> >>
> >>>> >> #define FF_ENCRYPTION_INIT_INFO_EXTRA 16
> >>>> >>
> >>>> >> @@ -215,6 +218,7 @@ void av_encryption_init_info_free(AVEncryptionInitInfo *info)
> >>>> >> for (i = 0; i < info->num_key_ids; i++) {
> >>>> >> av_free(info->key_ids[i]);
> >>>> >> }
> >>>> >> + av_encryption_init_info_free(info->next);
> >>>> >> av_free(info->system_id);
> >>>> >> av_free(info->key_ids);
> >>>> >> av_free(info->data);
> >>>> >> @@ -225,71 +229,115 @@ void av_encryption_init_info_free(AVEncryptionInitInfo *info)
> >>>> >> AVEncryptionInitInfo *av_encryption_init_info_get_side_data(
> >>>> >> const uint8_t *side_data, size_t side_data_size)
> >>>> >> {
> >>>> >> - AVEncryptionInitInfo *info;
> >>>> >> + AVEncryptionInitInfo *ret = NULL, *info;
> >>>> >> uint64_t system_id_size, num_key_ids, key_id_size, data_size, i;
> >>>> >> + uint64_t init_info_count;
> >>>> >>
> >>>> >> - if (!side_data || side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA)
> >>>> >> - return NULL;
> >>>> >> -
> >>>> >> - system_id_size = AV_RB32(side_data);
> >>>> > [...]
> >>>> >> + init_info_count = AV_RB32(side_data);
> >>>> >
> >>>> > i may be missing something but this looks like the meaning of the first
> >>>> > field changes, this thus doesnt look compatible to me
> >>>>
> >>>> It changes the binary format of the side-data, but that was explicitly
> >>>> not part of ABI. The fields in the structs are unchanged. This would
> >>>> only cause a problem if the side data bytes were stored out-of-band
> >>>> from a different version of FFmpeg.
> >>>
> >>> yes, right.
> >>> its side data that is neighter a C struct nor a well defined byte stream
> >>> its a opaque block that can only be passed to libavutil which then translates
> >>> it into a C struct.
> >>> its not new but it still feels clumsy to use this as means to pass data around
> >>>
> >>
> >> I know it's weird, but this was the design that was decided on when
> >> this was added. Are there any changes you want for this patch?
> >>
> >>>
> >>> [...]
> >>> --
> >>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >>>
> >>> If you think the mosad wants you dead since a long time then you are either
> >>> wrong or dead since a long time.
> >>>
> >>> _______________________________________________
> >>> ffmpeg-devel mailing list
> >>> ffmpeg-devel at ffmpeg.org
> >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>
> >
> > Ping
>
> Added fix for issue found by Chrome's ClusterFuzz (http://crbug.com/846662).
this belongs in a seperate patch unless its a bug specific to the code added
with this patch
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180526/d07cc10f/attachment.sig>
More information about the ffmpeg-devel
mailing list