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

2018-06-22 Thread Michael Niedermayer 
On Thu, Jun 21, 2018 at 12:01:09PM -0700, Jacob Trimble wrote:
> On Thu, Jun 21, 2018 at 9:48 AM Michael Niedermayer
>  wrote:
> >
> > > +if (UINT32_MAX == init_info_count ||
> > > +UINT32_MAX - *side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA 
> > > ||
> > > +UINT32_MAX - *side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA 
> > > < info->system_id_size ||
> > > +UINT32_MAX - *side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA 
> > > - info->system_id_size < info->data_size) {
> > >  return NULL;
> > > +}
> >
> > you can simplify this with (u)int64_t
> >
> 
> Done

>  encryption_info.c |  146 
> +++---
>  encryption_info.h |5 +
>  2 files changed, 100 insertions(+), 51 deletions(-)
> 676617bd11d32ea552e0fdb9fcdb2486241ab9f0  
> 0001-libavutil-encryption_info-Allow-multiple-init-info-8.patch
> From f440fe2be172672c439fa8b216b08a8d0895f76f Mon Sep 17 00:00:00 2001
> From: Jacob Trimble 
> 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 
> ---
>  libavutil/encryption_info.c | 146 +++-
>  libavutil/encryption_info.h |   5 ++
>  2 files changed, 100 insertions(+), 51 deletions(-)

will apply

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-06-21 Thread Jacob Trimble 
On Thu, Jun 21, 2018 at 9:48 AM Michael Niedermayer
 wrote:
>
> > +if (UINT32_MAX == init_info_count ||
> > +UINT32_MAX - *side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA ||
> > +UINT32_MAX - *side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < 
> > info->system_id_size ||
> > +UINT32_MAX - *side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA - 
> > info->system_id_size < info->data_size) {
> >  return NULL;
> > +}
>
> you can simplify this with (u)int64_t
>

Done
From f440fe2be172672c439fa8b216b08a8d0895f76f Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
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 
---
 libavutil/encryption_info.c | 146 +++-
 libavutil/encryption_info.h |   5 ++
 2 files changed, 100 insertions(+), 51 deletions(-)

diff --git a/libavutil/encryption_info.c b/libavutil/encryption_info.c
index 20a752d6b4..3b7e16cd0c 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,111 @@ 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;
-uint64_t system_id_size, num_key_ids, key_id_size, data_size, i;
+// |ret| tracks the front of the list, |info| tracks the back.
+AVEncryptionInitInfo *ret = NULL, *info, *temp_info;
+uint64_t system_id_size, num_key_ids, key_id_size, data_size, i, j;
+uint64_t init_info_count;
 
-if (!side_data || side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA)
+if (!side_data || side_data_size < 4)
 return NULL;
 
-system_id_size = AV_RB32(side_data);
-num_key_ids = AV_RB32(side_data + 4);
-key_id_size = AV_RB32(side_data + 8);
-data_size = AV_RB32(side_data + 12);
+init_info_count = AV_RB32(side_data);
+side_data += 4;
+side_data_size -= 4;
+for (i = 0; i < init_info_count; i++) {
+if (side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA) {
+av_encryption_init_info_free(ret);
+return NULL;
+}
 
-// UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
-if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size)
-return NULL;
+system_id_size = AV_RB32(side_data);
+num_key_ids = AV_RB32(side_data + 4);
+key_id_size = AV_RB32(side_data + 8);
+data_size = AV_RB32(side_data + 12);
 
-info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
-if (!info)
-return NULL;
+// UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
+if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size) {
+av_encryption_init_info_free(ret);
+return NULL;
+}
+side_data += FF_ENCRYPTION_INIT_INFO_EXTRA;
+side_data_size -= FF_ENCRYPTION_INIT_INFO_EXTRA;
+
+temp_info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
+if (!temp_info) {
+av_encryption_init_info_free(ret);
+return NULL;
+}
+if (i == 0) {
+info = ret = temp_info;
+} else {
+info->next = temp_info;
+info = temp_info;
+}
 
-memcpy(info->system_id, side_data + 16, system_id_size);
-side_data += system_id_size + 16;
-for (i = 0; i < num_key_ids; i++) {
-  memcpy(info->key_ids[i], side_data, key_id_size);
-  side_data +=

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

2018-06-21 Thread Michael Niedermayer 
On Thu, Jun 07, 2018 at 11:51:30AM -0700, Jacob Trimble wrote:
> On Thu, May 31, 2018 at 5:50 PM Jacob Trimble  wrote:
> >
> > On Thu, May 31, 2018 at 9:40 AM Jacob Trimble  wrote:
> > >
> > > On Fri, May 25, 2018 at 6:13 PM Michael Niedermayer
> > >  wrote:
> > > >
> > > > [...]
> > > >
> > > > > 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
> > > >
> > >
> > > Ok.  Now this patch depends on
> > > http://ffmpeg.org/pipermail/ffmpeg-devel/2018-May/230782.html.
> > >
> >
> > Noticed some bugs when integrating it.
> >
> > > > [...]
> > > >
> > > > --
> > > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> > > >
> > > > I have often repented speaking, but never of holding my tongue.
> > > > -- Xenocrates
> > > > ___
> > > > ffmpeg-devel mailing list
> > > > ffmpeg-devel@ffmpeg.org
> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> Removed controversial NULL checks.  This patch no longer depends on anything.

>  encryption_info.c |  140 
> --
>  encryption_info.h |5 +
>  2 files changed, 100 insertions(+), 45 deletions(-)
> cc25918cb63c4ac75f0b693472b51590c1a74052  
> 0001-libavutil-encryption_info-Allow-multiple-init-info-v7.patch
> From 0c4c33f26ba4204a775709cdd6367dd1ea4bd024 Mon Sep 17 00:00:00 2001
> From: Jacob Trimble 
> 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 
> ---
>  libavutil/encryption_info.c | 140 
>  libavutil/encryption_info.h |   5 ++
>  2 files changed, 100 insertions(+), 45 deletions(-)
> 
> diff --git a/libavutil/encryption_info.c b/libavutil/encryption_info.c
> index 20a752d6b4..1072d2795b 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,117 @@ 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;
> -uint64_t system_id_size, num_key_ids, key_id_size, data_size, i;
> +// |ret| tracks the front of the list, |info| tracks the back.
> +AVEncryptionInitInfo *ret = NULL, *info, *temp_info;
> +uint64_t system_id_size, num_key_ids, key_id_size, data_size, i, j;
> +uint64_t init_info_count;
>  
> -if (!side_data || side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA)
> +if (!side_data || side_data_size < 4)
>  return NULL;
>  
> -system_id_size = AV_RB32(side_data);
> -num_key_ids = AV_RB32(side_data + 4);
> -key_id_size = AV_RB32(side_data + 8);
> -data_size = AV_RB32(side_data + 12);
> +init_info_count = AV_RB32(side_data);
> +side_data += 4;
> +side_data_size -= 4;
> +for (i = 0; i < init_info_count; i++) {
> +if (side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA) {
> +av_encryption_init_info_free(ret);
> +return NULL;
> +}
>  
> -// UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
> -if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + 
> data_size + num_key_ids * key_id_size)
> -return NULL;
> +system_id_size = AV_RB32(side_data);
> +num_key_ids = AV_RB32(side_data + 4);
> +key_id_size = AV_RB32(side_data + 8);
> +data_size = AV_RB32(side_data + 12);
>  
> -info =

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

2018-06-20 Thread Jacob Trimble 
On Thu, Jun 14, 2018 at 9:44 AM Jacob Trimble  wrote:
>
> On Thu, Jun 7, 2018 at 11:51 AM Jacob Trimble  wrote:
> >
> > On Thu, May 31, 2018 at 5:50 PM Jacob Trimble  wrote:
> > >
> > > On Thu, May 31, 2018 at 9:40 AM Jacob Trimble  wrote:
> > > >
> > > > On Fri, May 25, 2018 at 6:13 PM Michael Niedermayer
> > > >  wrote:
> > > > >
> > > > > [...]
> > > > >
> > > > > > 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
> > > > >
> > > >
> > > > Ok.  Now this patch depends on
> > > > http://ffmpeg.org/pipermail/ffmpeg-devel/2018-May/230782.html.
> > > >
> > >
> > > Noticed some bugs when integrating it.
> > >
> > > > > [...]
> > > > >
> > > > > --
> > > > > Michael GnuPG fingerprint: 
> > > > > 9FF2128B147EF6730BADF133611EC787040B0FAB
> > > > >
> > > > > I have often repented speaking, but never of holding my tongue.
> > > > > -- Xenocrates
> > > > > ___
> > > > > ffmpeg-devel mailing list
> > > > > ffmpeg-devel@ffmpeg.org
> > > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > Removed controversial NULL checks.  This patch no longer depends on 
> > anything.
>
> Ping.

Ping.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-06-14 Thread Jacob Trimble 
On Thu, Jun 7, 2018 at 11:51 AM Jacob Trimble  wrote:
>
> On Thu, May 31, 2018 at 5:50 PM Jacob Trimble  wrote:
> >
> > On Thu, May 31, 2018 at 9:40 AM Jacob Trimble  wrote:
> > >
> > > On Fri, May 25, 2018 at 6:13 PM Michael Niedermayer
> > >  wrote:
> > > >
> > > > [...]
> > > >
> > > > > 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
> > > >
> > >
> > > Ok.  Now this patch depends on
> > > http://ffmpeg.org/pipermail/ffmpeg-devel/2018-May/230782.html.
> > >
> >
> > Noticed some bugs when integrating it.
> >
> > > > [...]
> > > >
> > > > --
> > > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> > > >
> > > > I have often repented speaking, but never of holding my tongue.
> > > > -- Xenocrates
> > > > ___
> > > > ffmpeg-devel mailing list
> > > > ffmpeg-devel@ffmpeg.org
> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> Removed controversial NULL checks.  This patch no longer depends on anything.

Ping.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-06-07 Thread Jacob Trimble 
On Thu, May 31, 2018 at 5:50 PM Jacob Trimble  wrote:
>
> On Thu, May 31, 2018 at 9:40 AM Jacob Trimble  wrote:
> >
> > On Fri, May 25, 2018 at 6:13 PM Michael Niedermayer
> >  wrote:
> > >
> > > [...]
> > >
> > > > 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
> > >
> >
> > Ok.  Now this patch depends on
> > http://ffmpeg.org/pipermail/ffmpeg-devel/2018-May/230782.html.
> >
>
> Noticed some bugs when integrating it.
>
> > > [...]
> > >
> > > --
> > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> > >
> > > I have often repented speaking, but never of holding my tongue.
> > > -- Xenocrates
> > > ___
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel@ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Removed controversial NULL checks.  This patch no longer depends on anything.
From 0c4c33f26ba4204a775709cdd6367dd1ea4bd024 Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
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 
---
 libavutil/encryption_info.c | 140 
 libavutil/encryption_info.h |   5 ++
 2 files changed, 100 insertions(+), 45 deletions(-)

diff --git a/libavutil/encryption_info.c b/libavutil/encryption_info.c
index 20a752d6b4..1072d2795b 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,117 @@ 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;
-uint64_t system_id_size, num_key_ids, key_id_size, data_size, i;
+// |ret| tracks the front of the list, |info| tracks the back.
+AVEncryptionInitInfo *ret = NULL, *info, *temp_info;
+uint64_t system_id_size, num_key_ids, key_id_size, data_size, i, j;
+uint64_t init_info_count;
 
-if (!side_data || side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA)
+if (!side_data || side_data_size < 4)
 return NULL;
 
-system_id_size = AV_RB32(side_data);
-num_key_ids = AV_RB32(side_data + 4);
-key_id_size = AV_RB32(side_data + 8);
-data_size = AV_RB32(side_data + 12);
+init_info_count = AV_RB32(side_data);
+side_data += 4;
+side_data_size -= 4;
+for (i = 0; i < init_info_count; i++) {
+if (side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA) {
+av_encryption_init_info_free(ret);
+return NULL;
+}
 
-// UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
-if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size)
-return NULL;
+system_id_size = AV_RB32(side_data);
+num_key_ids = AV_RB32(side_data + 4);
+key_id_size = AV_RB32(side_data + 8);
+data_size = AV_RB32(side_data + 12);
 
-info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
-if (!info)
-return NULL;
+// UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
+if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size) {
+av_encryption_init_info_free(ret);
+return NULL;
+}
+side_data += FF_ENCRYPTION_INIT_INFO_EXTRA;
+side_data_size -= FF_ENCRYPTION_INIT_INFO_EXTRA;
 
-memcpy(info->system_id, side_data + 16, system_id_size);
-side_data += system_id_size +

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

2018-05-31 Thread Jacob Trimble 
On Thu, May 31, 2018 at 9:40 AM Jacob Trimble  wrote:
>
> On Fri, May 25, 2018 at 6:13 PM Michael Niedermayer
>  wrote:
> >
> > [...]
> >
> > > 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
> >
>
> Ok.  Now this patch depends on
> http://ffmpeg.org/pipermail/ffmpeg-devel/2018-May/230782.html.
>

Noticed some bugs when integrating it.

> > [...]
> >
> > --
> > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > I have often repented speaking, but never of holding my tongue.
> > -- Xenocrates
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
From ea52a1264787256a2c92bc162d217b98e1b39e23 Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
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 
---
 libavutil/encryption_info.c | 153 
 libavutil/encryption_info.h |   5 ++
 2 files changed, 109 insertions(+), 49 deletions(-)

diff --git a/libavutil/encryption_info.c b/libavutil/encryption_info.c
index a48ded922c..00f7d0fde0 100644
--- a/libavutil/encryption_info.c
+++ b/libavutil/encryption_info.c
@@ -162,13 +162,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
 
@@ -217,6 +220,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);
@@ -227,72 +231,123 @@ 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;
-uint64_t system_id_size, num_key_ids, key_id_size, data_size, i;
+// |ret| tracks the front of the list, |info| tracks the back.
+AVEncryptionInitInfo *ret = NULL, *info, *temp_info;
+uint64_t system_id_size, num_key_ids, key_id_size, data_size, i, j;
+uint64_t init_info_count;
 
-if (!side_data || side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA)
+if (!side_data || side_data_size < 4)
 return NULL;
 
-system_id_size = AV_RB32(side_data);
-num_key_ids = AV_RB32(side_data + 4);
-key_id_size = AV_RB32(side_data + 8);
-data_size = AV_RB32(side_data + 12);
+init_info_count = AV_RB32(side_data);
+side_data += 4;
+side_data_size -= 4;
+for (i = 0; i < init_info_count; i++) {
+if (side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA) {
+av_encryption_init_info_free(ret);
+return NULL;
+}
 
-// UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
-if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size)
-return NULL;
+system_id_size = AV_RB32(side_data);
+num_key_ids = AV_RB32(side_data + 4);
+key_id_size = AV_RB32(side_data + 8);
+data_size = AV_RB32(side_data + 12);
 
-info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
-if (!info)
-return NULL;
+// UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
+if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size) {
+av_encryption_init_info_free(ret);
+return NULL;
+}
+side_data += FF_ENCRYPTION_INIT_INFO_EXTRA;
+side_data_size -= FF_ENCRYPTION_INIT_INFO_EXTRA;
+
+temp_info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
+if (!temp_info) {
+av_encryption_init_info_free(ret);
+return NULL;
+}
+if (i == 0) {
+info = ret = temp_info;
+} else {
+

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

2018-05-31 Thread Jacob Trimble 
On Fri, May 25, 2018 at 6:13 PM Michael Niedermayer
 wrote:
>
> [...]
>
> > 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
>

Ok.  Now this patch depends on
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-May/230782.html.

> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I have often repented speaking, but never of holding my tongue.
> -- Xenocrates
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
From 468958a26c1df8f599925038518ca4dae90de0f7 Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
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 
---
 libavutil/encryption_info.c | 148 
 libavutil/encryption_info.h |   5 ++
 2 files changed, 104 insertions(+), 49 deletions(-)

diff --git a/libavutil/encryption_info.c b/libavutil/encryption_info.c
index a48ded922c..4675ceccf6 100644
--- a/libavutil/encryption_info.c
+++ b/libavutil/encryption_info.c
@@ -162,13 +162,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
 
@@ -217,6 +220,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);
@@ -227,72 +231,118 @@ 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;
-uint64_t system_id_size, num_key_ids, key_id_size, data_size, i;
+AVEncryptionInitInfo *ret = NULL, *info;
+uint64_t system_id_size, num_key_ids, key_id_size, data_size, i, j;
+uint64_t init_info_count;
 
-if (!side_data || side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA)
+if (!side_data || side_data_size < 4)
 return NULL;
 
-system_id_size = AV_RB32(side_data);
-num_key_ids = AV_RB32(side_data + 4);
-key_id_size = AV_RB32(side_data + 8);
-data_size = AV_RB32(side_data + 12);
+init_info_count = AV_RB32(side_data);
+side_data += 4;
+side_data_size -= 4;
+for (i = 0; i < init_info_count; i++) {
+if (side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA) {
+av_encryption_init_info_free(ret);
+return NULL;
+}
 
-// UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
-if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size)
-return NULL;
+system_id_size = AV_RB32(side_data);
+num_key_ids = AV_RB32(side_data + 4);
+key_id_size = AV_RB32(side_data + 8);
+data_size = AV_RB32(side_data + 12);
 
-info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
-if (!info)
-return NULL;
+// UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
+if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size) {
+av_encryption_init_info_free(ret);
+return NULL;
+}
+side_data += FF_ENCRYPTION_INIT_INFO_EXTRA;
+side_data_size -= FF_ENCRYPTION_INIT_INFO_EXTRA;
 
-memcpy(info->system_id, side_data + 16, system_id_size);
-side_data += system_id_size + 16;
-for (i = 0; i < num_key_ids; i++) {
-  memcpy(info->key_ids[i], side_data, key_id_size);
-  side_data += key_id_size;
+info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
+if (!info) {
+av_encryption_init_info_free(ret);
+return NULL;
+}
+info->next = ret;
+ret = info;
+
+memcpy(info->system_id, side_data,

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

2018-05-25 Thread Michael Niedermayer 
On Fri, May 25, 2018 at 02:16:47PM -0700, Jacob Trimble wrote:
> On Mon, May 21, 2018 at 9:25 AM, Jacob Trimble  wrote:
> > On Mon, May 14, 2018 at 4:49 PM, Jacob Trimble  wrote:
> >> On Tue, May 8, 2018 at 3:47 PM, Michael Niedermayer
> >>  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
>   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 
>  >> 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 
>  >> ---
>  >>  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

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

2018-05-25 Thread Jacob Trimble 
On Mon, May 21, 2018 at 9:25 AM, Jacob Trimble  wrote:
> On Mon, May 14, 2018 at 4:49 PM, Jacob Trimble  wrote:
>> On Tue, May 8, 2018 at 3:47 PM, Michael Niedermayer
>>  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
  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 
 >> 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 
 >> ---
 >>  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

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

2018-05-21 Thread Jacob Trimble 
On Mon, May 14, 2018 at 4:49 PM, Jacob Trimble  wrote:
> On Tue, May 8, 2018 at 3:47 PM, Michael Niedermayer
>  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
>>>  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 
>>> >> 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 
>>> >> ---
>>> >>  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

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

2018-05-14 Thread Jacob Trimble 
On Tue, May 8, 2018 at 3:47 PM, Michael Niedermayer
 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
>>  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 
>> >> 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 
>> >> ---
>> >>  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

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

2018-05-08 Thread Michael Niedermayer 
On Mon, May 07, 2018 at 04:59:33PM -0700, Jacob Trimble wrote:
> On Mon, May 7, 2018 at 3:18 PM, Michael Niedermayer
>  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 
> >> 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 
> >> ---
> >>  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

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

2018-05-07 Thread Jacob Trimble 
On Mon, May 7, 2018 at 3:18 PM, Michael Niedermayer
 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 
>> 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 
>> ---
>>  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@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
From 039632f7b75da569f3e43780a9fa36454031f37e Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
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

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

2018-05-07 Thread Michael Niedermayer 
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 
> 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 
> ---
>  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

also indention is quite inconsistent in the new code

[...]

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


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-05-07 Thread Jacob Trimble 
On Fri, Apr 27, 2018 at 5:30 PM, Jacob Trimble  wrote:
> On Fri, Apr 27, 2018 at 10:33 AM, Jacob Trimble  wrote:
>> On Mon, Apr 23, 2018 at 11:03 AM, 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.
>>
>> Ping
>
> Noticed a bug, fixed.

Ping.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-04-27 Thread Jacob Trimble 
On Fri, Apr 27, 2018 at 10:33 AM, Jacob Trimble  wrote:
> On Mon, Apr 23, 2018 at 11:03 AM, 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.
>
> Ping

Noticed a bug, fixed.
From 50ec8ed80687134ecb9e74c45336e920d9e28666 Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
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 
---
 libavutil/encryption_info.c | 156 +++-
 libavutil/encryption_info.h |   5 ++
 2 files changed, 107 insertions(+), 54 deletions(-)

diff --git a/libavutil/encryption_info.c b/libavutil/encryption_info.c
index 20a752d6b4..2329d94f12 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;
-uint64_t system_id_size, num_key_ids, key_id_size, data_size, i;
-
-if (!side_data || side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA)
-return NULL;
-
-system_id_size = AV_RB32(side_data);
-num_key_ids = AV_RB32(side_data + 4);
-key_id_size = AV_RB32(side_data + 8);
-data_size = AV_RB32(side_data + 12);
-
-// UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
-if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size)
-return NULL;
+AVEncryptionInitInfo *ret = NULL, *info;
+uint64_t system_id_size, num_key_ids, key_id_size, data_size, i, j;
+uint64_t init_info_count;
 
-info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
-if (!info)
+if (!side_data || side_data_size < 4)
 return NULL;
 
-memcpy(info->system_id, side_data + 16, system_id_size);
-side_data += system_id_size + 16;
-for (i = 0; i < num_key_ids; i++) {
-  memcpy(info->key_ids[i], side_data, key_id_size);
-  side_data += key_id_size;
+init_info_count = AV_RB32(side_data);
+side_data += 4;
+side_data_size -= 4;
+for (i = 0; i < init_info_count; i++) {
+  if (side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA) {
+  av_encryption_init_info_free(ret);
+  return NULL;
+  }
+
+  system_id_size = AV_RB32(side_data);
+  num_key_ids = AV_RB32(side_data + 4);
+  key_id_size = AV_RB32(side_data + 8);
+  data_size = AV_RB32(side_data + 12);
+
+  // UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
+  if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size) {
+  av_encryption_init_info_free(ret);
+  return NULL;
+  }
+  side_data += FF_ENCRYPTION_INIT_INFO_EXTRA;
+  side_data_size -= FF_ENCRYPTION_INIT_INFO_EXTRA;
+
+  info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
+  if

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

2018-04-27 Thread Jacob Trimble 
On Mon, Apr 23, 2018 at 11:03 AM, 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.

Ping
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-04-23 Thread Jacob Trimble 
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.
From bb941a77e882e93629d63d63059d0063b9519e29 Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
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 
---
 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);
-num_key_ids = AV_RB32(side_data + 4);
-key_id_size = AV_RB32(side_data + 8);
-data_size = AV_RB32(side_data + 12);
-
-// UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
-if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size)
-return NULL;
-
-info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
-if (!info)
+if (!side_data || side_data_size < 4)
 return NULL;
 
-memcpy(info->system_id, side_data + 16, system_id_size);
-side_data += system_id_size + 16;
-for (i = 0; i < num_key_ids; i++) {
-  memcpy(info->key_ids[i], side_data, key_id_size);
-  side_data += key_id_size;
+init_info_count = AV_RB32(side_data);
+side_data += 4;
+side_data_size -= 4;
+for (i = 0; i < init_info_count; i++) {
+  if (side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA) {
+  av_encryption_init_info_free(ret);
+  return NULL;
+  }
+
+  system_id_size = AV_RB32(side_data);
+  num_key_ids = AV_RB32(side_data + 4);
+  key_id_size = AV_RB32(side_data + 8);
+  data_size = AV_RB32(side_data + 12);
+
+  // UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
+  if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size) {
+  av_encryption_init_info_free(ret);
+  return NULL;
+  }
+  side_data += FF_ENCRYPTION_INIT_INFO_EXTRA;
+  side_data_size -= FF_ENCRYPTION_INIT_INFO_EXTRA;
+
+  info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
+  if (!info) {
+  av_encryption_init_info_free(ret);
+  return NULL;
+  }
+  info->next = ret;
+  ret = info;
+
+  memcpy(info->system_id, side_data, system_id_size);
+  side_data += system_id_size;
+  side_data_size -= system_id_size;
+  for (i = 0; i <