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

[Jacob Trimble]

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.

>
> also indention is quite inconsistent in the new code

One of these days, I'll remember that this uses 4 spaces not 2.  Fixed.

>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Many that live deserve death. And some that die deserve life. Can you give
> it to them? Then do not be too eager to deal out death in judgement. For
> even the very wise cannot see all ends. -- Gandalf
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-libavutil-encryption_info-Allow-multiple-init-info-v3.patch
Type: text/x-patch
Size: 8799 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180507/268cbcbf/attachment.bin>