On Mon, May 07, 2018 at 04:59:33PM -0700, Jacob Trimble wrote: > On Mon, May 7, 2018 at 3:18 PM, Michael Niedermayer > <mich...@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 <modma...@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 <modma...@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 [...] -- 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.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel