[FFmpeg-devel] libavutil/encryption_info: Allow multiple init info.

Jacob Trimble modmaker at google.com
Sat May 26 00:16:47 EEST 2018


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).

PTAL.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-libavutil-encryption_info-Allow-multiple-init-info-v4.patch
Type: application/octet-stream
Size: 9666 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180525/4f00e0f0/attachment.obj>


More information about the ffmpeg-devel mailing list