Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Michael Niedermayer]

On Mon, Jan 08, 2018 at 05:03:32PM -0800, Jacob Trimble wrote:
> On Mon, Jan 8, 2018 at 11:40 AM, Jacob Trimble  wrote:
> >> I'd assume we'd wait with applying this until the mp4 patch that uses
> >> it is reviewed. I'm fine with this patch and I think it can be pushed
> >> as it is, although I just noticed an APIchanges entry and minor version
> >> bump is actually missing. You could add them when sending the final
> >> patch set.
> >
> > Added APIchanges entry and version bump.
> >
> >> The format of teh side data should be choosen so that demuxers and encoders
> >> producing it can interoperate with muxers and decoders consuming it.
> >
> >> Please correct me if iam wrong but if some containers can store more than
> >> others, then the "other" containers would not use that extra information 
> >> nor
> >> set it but it could still be using a compatible representation for 
> >> interoperability
> >
> > Well I guess I can do the same thing for this as for the
> > frame-specific encryption info.  There will be a user-friendly struct
> > for accessing it and a binary format that will be stored in the side
> > data.  This will allow different (de)muxers to use the data in a
> > generic way.
> >
> >> These should be checked against size, making sure the data is consistent
> >
> > Done.  Also added addition overflow checking.
> >
> > On Fri, Jan 5, 2018 at 2:42 PM, James Almer  wrote:
> >> This being in libavutil, the two functions using AVPacket should be
> >> changed to take and return a byte array instead, that the caller
> >> manually either got from side data, or will afterwards adds as side data.
> >
> > Done.
> 
> Noticed some bugs when integrating with my other changes.

>  doc/APIchanges  |4 
>  libavcodec/avcodec.h|   13 +
>  libavutil/Makefile  |2 
>  libavutil/encryption_info.c |  295 
> 
>  libavutil/encryption_info.h |  200 +
>  libavutil/version.h |2 
>  6 files changed, 515 insertions(+), 1 deletion(-)
> d8243c445d7bb638a1599ca9b847674a925578b8  encryption-info-v9.patch
> From 54f996fd8b8b8e44506739b58784e6038309 Mon Sep 17 00:00:00 2001
> From: Jacob Trimble 
> Date: Tue, 5 Dec 2017 14:52:22 -0800
> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
> 
> This new side-data will contain info on how a packet is encrypted.
> This allows the app to handle packet decryption.

will apply, as this is blocking other patches and noone seems to have
objected

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein


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

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Jacob Trimble]

On Mon, Jan 8, 2018 at 11:40 AM, Jacob Trimble  wrote:
>> I'd assume we'd wait with applying this until the mp4 patch that uses
>> it is reviewed. I'm fine with this patch and I think it can be pushed
>> as it is, although I just noticed an APIchanges entry and minor version
>> bump is actually missing. You could add them when sending the final
>> patch set.
>
> Added APIchanges entry and version bump.
>
>> The format of teh side data should be choosen so that demuxers and encoders
>> producing it can interoperate with muxers and decoders consuming it.
>
>> Please correct me if iam wrong but if some containers can store more than
>> others, then the "other" containers would not use that extra information nor
>> set it but it could still be using a compatible representation for 
>> interoperability
>
> Well I guess I can do the same thing for this as for the
> frame-specific encryption info.  There will be a user-friendly struct
> for accessing it and a binary format that will be stored in the side
> data.  This will allow different (de)muxers to use the data in a
> generic way.
>
>> These should be checked against size, making sure the data is consistent
>
> Done.  Also added addition overflow checking.
>
> On Fri, Jan 5, 2018 at 2:42 PM, James Almer  wrote:
>> This being in libavutil, the two functions using AVPacket should be
>> changed to take and return a byte array instead, that the caller
>> manually either got from side data, or will afterwards adds as side data.
>
> Done.

Noticed some bugs when integrating with my other changes.
From 54f996fd8b8b8e44506739b58784e6038309 Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
Date: Tue, 5 Dec 2017 14:52:22 -0800
Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.

This new side-data will contain info on how a packet is encrypted.
This allows the app to handle packet decryption.

Signed-off-by: Jacob Trimble 
---
 doc/APIchanges  |   4 +
 libavcodec/avcodec.h|  13 ++
 libavutil/Makefile  |   2 +
 libavutil/encryption_info.c | 295 
 libavutil/encryption_info.h | 200 ++
 libavutil/version.h |   2 +-
 6 files changed, 515 insertions(+), 1 deletion(-)
 create mode 100644 libavutil/encryption_info.c
 create mode 100644 libavutil/encryption_info.h

diff --git a/doc/APIchanges b/doc/APIchanges
index d66c842521..b771031740 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,10 @@ libavutil: 2017-10-21
 
 API changes, most recent first:
 
+2018-xx-xx - xxx - lavu 56.8.100 - encryption_info.h
+  Add AVEncryptionInitInfo and AVEncryptionInfo structures to hold new side-data
+  for encryption info.
+
 2018-01-xx - xxx - lavfi 7.11.101 - avfilter.h
   Deprecate avfilter_link_get_channels(). Use av_buffersink_get_channels().
 
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index c13deb599f..2e5d39eae3 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1341,6 +1341,19 @@ enum AVPacketSideDataType {
  */
 AV_PKT_DATA_A53_CC,
 
+/**
+ * This side data is encryption initialization data.
+ * The format is not part of ABI, use av_encryption_init_info_* methods to
+ * access.
+ */
+AV_PKT_DATA_ENCRYPTION_INIT_INFO,
+
+/**
+ * This side data contains encryption info for how to decrypt the packet.
+ * The format is not part of ABI, use av_encryption_info_* methods to access.
+ */
+AV_PKT_DATA_ENCRYPTION_INFO,
+
 /**
  * The number of side data types.
  * This is not part of the public API/ABI in the sense that it may
diff --git a/libavutil/Makefile b/libavutil/Makefile
index d7474f59e4..760fce3b30 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -24,6 +24,7 @@ HEADERS = adler32.h \
   dict.h\
   display.h \
   downmix_info.h\
+  encryption_info.h \
   error.h   \
   eval.h\
   fifo.h\
@@ -107,6 +108,7 @@ OBJS = adler32.o\
dict.o   \
display.o\
downmix_info.o   \
+   encryption_info.o\
error.o  \
eval.o 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Jacob Trimble]

> I'd assume we'd wait with applying this until the mp4 patch that uses
> it is reviewed. I'm fine with this patch and I think it can be pushed
> as it is, although I just noticed an APIchanges entry and minor version
> bump is actually missing. You could add them when sending the final
> patch set.

Added APIchanges entry and version bump.

> The format of teh side data should be choosen so that demuxers and encoders
> producing it can interoperate with muxers and decoders consuming it.

> Please correct me if iam wrong but if some containers can store more than
> others, then the "other" containers would not use that extra information nor
> set it but it could still be using a compatible representation for 
> interoperability

Well I guess I can do the same thing for this as for the
frame-specific encryption info.  There will be a user-friendly struct
for accessing it and a binary format that will be stored in the side
data.  This will allow different (de)muxers to use the data in a
generic way.

> These should be checked against size, making sure the data is consistent

Done.  Also added addition overflow checking.

On Fri, Jan 5, 2018 at 2:42 PM, James Almer  wrote:
> This being in libavutil, the two functions using AVPacket should be
> changed to take and return a byte array instead, that the caller
> manually either got from side data, or will afterwards adds as side data.

Done.
From 332f20f951fc4db77a83f9dc60f75e84717b4fd0 Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
Date: Tue, 5 Dec 2017 14:52:22 -0800
Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.

This new side-data will contain info on how a packet is encrypted.
This allows the app to handle packet decryption.

Signed-off-by: Jacob Trimble 
---
 doc/APIchanges  |   4 +
 libavcodec/avcodec.h|  13 ++
 libavutil/Makefile  |   2 +
 libavutil/encryption_info.c | 295 
 libavutil/encryption_info.h | 200 ++
 libavutil/version.h |   2 +-
 6 files changed, 515 insertions(+), 1 deletion(-)
 create mode 100644 libavutil/encryption_info.c
 create mode 100644 libavutil/encryption_info.h

diff --git a/doc/APIchanges b/doc/APIchanges
index d66c842521..b771031740 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,10 @@ libavutil: 2017-10-21
 
 API changes, most recent first:
 
+2018-xx-xx - xxx - lavu 56.8.100 - encryption_info.h
+  Add AVEncryptionInitInfo and AVEncryptionInfo structures to hold new side-data
+  for encryption info.
+
 2018-01-xx - xxx - lavfi 7.11.101 - avfilter.h
   Deprecate avfilter_link_get_channels(). Use av_buffersink_get_channels().
 
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index c13deb599f..cc74a9a740 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1341,6 +1341,19 @@ enum AVPacketSideDataType {
  */
 AV_PKT_DATA_A53_CC,
 
+/**
+ * This side data is encryption initialization data.
+ * The format is not part of ABI, use av_encryption_init_info_* methods to
+ * access.
+ */
+AV_PKT_DATA_ENCRYPTION_INIT_DATA,
+
+/**
+ * This side data contains encryption info for how to decrypt the packet.
+ * The format is not part of ABI, use av_encryption_info_* methods to access.
+ */
+AV_PKT_DATA_ENCRYPTION_INFO,
+
 /**
  * The number of side data types.
  * This is not part of the public API/ABI in the sense that it may
diff --git a/libavutil/Makefile b/libavutil/Makefile
index d7474f59e4..760fce3b30 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -24,6 +24,7 @@ HEADERS = adler32.h \
   dict.h\
   display.h \
   downmix_info.h\
+  encryption_info.h \
   error.h   \
   eval.h\
   fifo.h\
@@ -107,6 +108,7 @@ OBJS = adler32.o\
dict.o   \
display.o\
downmix_info.o   \
+   encryption_info.o\
error.o  \
eval.o   \
fifo.o   \
diff --git a/libavutil/encryption_info.c b/libavutil/encryption_info.c
new 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[James Almer]

On 1/5/2018 7:32 PM, Michael Niedermayer wrote:
> On Fri, Jan 05, 2018 at 08:27:41PM +0100, wm4 wrote:
>> On Fri, 5 Jan 2018 10:22:31 -0800
>> Jacob Trimble  wrote:
>>
>>> Just noticed the new files were in libavcodec, moved to libavutil.
>>> Can someone please review this so it can be pushed.  I have the MP4
>>> implementation ready and I would like to get that reviewed and pushed
>>> as soon as possible.
>>
>> I'd assume we'd wait with applying this until the mp4 patch that uses
>> it is reviewed. I'm fine with this patch and I think it can be pushed
>> as it is, although I just noticed an APIchanges entry and minor version
>> bump is actually missing. You could add them when sending the final
>> patch set.
>>
> 
>> Maybe getting confirmation from Michael Niedermayer that it's safe to
>> proceed would be good (i.e. that the patch will be accepted without
>> major changes).
> 
> if everyone is ok with the design and no regressions are found than iam ok
> with it as well

This being in libavutil, the two functions using AVPacket should be
changed to take and return a byte array instead, that the caller
manually either got from side data, or will afterwards adds as side data.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Michael Niedermayer]

On Fri, Jan 05, 2018 at 08:27:41PM +0100, wm4 wrote:
> On Fri, 5 Jan 2018 10:22:31 -0800
> Jacob Trimble  wrote:
> 
> > Just noticed the new files were in libavcodec, moved to libavutil.
> > Can someone please review this so it can be pushed.  I have the MP4
> > implementation ready and I would like to get that reviewed and pushed
> > as soon as possible.
> 
> I'd assume we'd wait with applying this until the mp4 patch that uses
> it is reviewed. I'm fine with this patch and I think it can be pushed
> as it is, although I just noticed an APIchanges entry and minor version
> bump is actually missing. You could add them when sending the final
> patch set.
> 

> Maybe getting confirmation from Michael Niedermayer that it's safe to
> proceed would be good (i.e. that the patch will be accepted without
> major changes).

if everyone is ok with the design and no regressions are found than iam ok
with it as well


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope


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

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Michael Niedermayer]

On Fri, Jan 05, 2018 at 10:22:31AM -0800, Jacob Trimble wrote:
> On Tue, Jan 2, 2018 at 9:57 AM, Jacob Trimble  wrote:
> > On Wed, Dec 20, 2017 at 4:31 PM, wm4  wrote:
> >> On Wed, 20 Dec 2017 15:10:43 -0800
> >> Jacob Trimble  wrote:
> >>
> >>> From 1508d19e9f7acf43d76010ce54d59ff204613601 Mon Sep 17 00:00:00 2001
> >>> From: Jacob Trimble 
> >>> Date: Tue, 5 Dec 2017 14:52:22 -0800
> >>> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
> >>>
> >>> This new side-data will contain info on how a packet is encrypted.
> >>> This allows the app to handle packet decryption.
> >>>
> >>> Signed-off-by: Jacob Trimble 
> >>> ---
> >>
> >> Looks generally good to me, a few minor cosmetic comments below.
> >>
> >>>  libavcodec/Makefile  |   2 +
> >>>  libavcodec/avcodec.h |  13 
> >>>  libavcodec/encryption_info.c | 139 
> >>> +++
> >>>  libavcodec/encryption_info.h | 121 +
> >>>  4 files changed, 275 insertions(+)
> >>>  create mode 100644 libavcodec/encryption_info.c
> >>>  create mode 100644 libavcodec/encryption_info.h
> >>>
> >>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> >>> index ab7893f560..11ad642c6c 100644
> >>> --- a/libavcodec/Makefile
> >>> +++ b/libavcodec/Makefile
> >>> @@ -10,6 +10,7 @@ HEADERS = ac3_parser.h  
> >>> \
> >>>dirac.h   \
> >>>dv_profile.h  \
> >>>dxva2.h   \
> >>> +  encryption_info.h \
> >>>jni.h \
> >>>mediacodec.h  \
> >>>qsv.h \
> >>> @@ -36,6 +37,7 @@ OBJS = ac3_parser.o 
> >>> \
> >>> dirac.o  \
> >>> dv_profile.o \
> >>> encode.o \
> >>> +   encryption_info.o\
> >>> imgconvert.o \
> >>> jni.o\
> >>> mathtables.o \
> >>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> >>> index 5db6a81320..b43638ebc5 100644
> >>> --- a/libavcodec/avcodec.h
> >>> +++ b/libavcodec/avcodec.h
> >>> @@ -1327,6 +1327,19 @@ enum AVPacketSideDataType {
> >>>   */
> >>>  AV_PKT_DATA_A53_CC,
> >>>
> >>> +/**
> >>> + * This side data is encryption "initialization data".
> >>> + * For MP4 this is the entire 'pssh' box.
> >>> + * For WebM this is the key ID.
> >>> + */
> >>> +AV_PKT_DATA_ENCRYPTION_INIT_DATA,
> >>> +
> >>> +/**
> >>> + * This side data contains encryption info for how to decrypt the 
> >>> packet.
> >>> + * The format is not part of ABI, use av_encryption_info_* methods 
> >>> to access.
> >>> + */
> >>> +AV_PKT_DATA_ENCRYPTION_INFO,
> >>> +
> >>>  /**
> >>>   * The number of side data types.
> >>>   * This is not part of the public API/ABI in the sense that it may
> >>> diff --git a/libavcodec/encryption_info.c b/libavcodec/encryption_info.c
> >>> new file mode 100644
> >>> index 00..59ee4c41a9
> >>> --- /dev/null
> >>> +++ b/libavcodec/encryption_info.c
> >>> @@ -0,0 +1,139 @@
> >>> +/**
> >>> + * This file is part of FFmpeg.
> >>> + *
> >>> + * FFmpeg is free software; you can redistribute it and/or
> >>> + * modify it under the terms of the GNU Lesser General Public
> >>> + * License as published by the Free Software Foundation; either
> >>> + * version 2.1 of the License, or (at your option) any later version.
> >>> + *
> >>> + * FFmpeg is distributed in the hope that it will be useful,
> >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >>> + * Lesser General Public License for more details.
> >>> + *
> >>> + * You should have received a copy of the GNU Lesser General Public
> >>> + * License along with FFmpeg; if not, write to the Free Software
> >>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
> >>> 02110-1301 USA
> >>> + */
> >>> +
> >>> +#include "encryption_info.h"
> >>> +#include "libavutil/avassert.h"
> >>> +#include "libavutil/intreadwrite.h"
> >>> +
> >>> +#define FF_ENCRYPTION_INFO_EXTRA 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Michael Niedermayer]

On Mon, Dec 18, 2017 at 11:17:48AM -0800, Jacob Trimble wrote:
> >> @@ -1327,6 +1384,19 @@ enum AVPacketSideDataType {
> >>   */
> >>  AV_PKT_DATA_A53_CC,
> >>
> >> +/**
> >> + * This side data is encryption "initialization data".
> >> + * For MP4 this is the entire 'pssh' box.
> >> + * For WebM this is the key ID.
> >> + */
> >> +AV_PKT_DATA_ENCRYPTION_INIT_DATA,
> >
> > So its basically like extradata is for a codec ?
> > If so it should be defined similarly as opaque encryption scheme specific 
> > data.
> > It should not be container specific.
> > Data taken from one container should be storable in another if both support
> > the features used
> 
> Yes, that's the idea.  Unfortunately the data is specific to a
> container.  This isn't taken from the common encryption spec, but from
> the EME spec.  The "EME initialization data registry" specifies
> several fixed formats for initialization data that are all specific to
> a container.  
> 
> It would be possible to parse the PSSH boxes and just push the generic
> data to the app.  This would allow new containers to hold the same
> data; then the app could wrap it back in a generic PSSH box for EME.
> But that would remove the key ID information that exists in v1 PSSH
> boxes.  Some apps parse the PSSH boxes and filter the initialization
> based on whether they already have those key IDs to avoid creating too
> many sessions.  So I would like to expose the whole PSSH box in the
> side data so apps can do that.

The format of teh side data should be choosen so that demuxers and encoders
producing it can interoperate with muxers and decoders consuming it.

Please correct me if iam wrong but if some containers can store more than
others, then the "other" containers would not use that extra information nor
set it but it could still be using a compatible representation for 
interoperability


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle


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

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[wm4]

On Fri, 5 Jan 2018 10:22:31 -0800
Jacob Trimble  wrote:

> Just noticed the new files were in libavcodec, moved to libavutil.
> Can someone please review this so it can be pushed.  I have the MP4
> implementation ready and I would like to get that reviewed and pushed
> as soon as possible.

I'd assume we'd wait with applying this until the mp4 patch that uses
it is reviewed. I'm fine with this patch and I think it can be pushed
as it is, although I just noticed an APIchanges entry and minor version
bump is actually missing. You could add them when sending the final
patch set.

Maybe getting confirmation from Michael Niedermayer that it's safe to
proceed would be good (i.e. that the patch will be accepted without
major changes).

It's also possible that others are fine with pushing it now, but I'd
argue it should wait until something uses it.

(Personally I don't really appreciate the complexity of exporting this
metadata, when libavformat can already decrypt the data internally. But
I suppose it has valid use cases.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Jacob Trimble]

On Tue, Jan 2, 2018 at 9:57 AM, Jacob Trimble  wrote:
> On Wed, Dec 20, 2017 at 4:31 PM, wm4  wrote:
>> On Wed, 20 Dec 2017 15:10:43 -0800
>> Jacob Trimble  wrote:
>>
>>> From 1508d19e9f7acf43d76010ce54d59ff204613601 Mon Sep 17 00:00:00 2001
>>> From: Jacob Trimble 
>>> Date: Tue, 5 Dec 2017 14:52:22 -0800
>>> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
>>>
>>> This new side-data will contain info on how a packet is encrypted.
>>> This allows the app to handle packet decryption.
>>>
>>> Signed-off-by: Jacob Trimble 
>>> ---
>>
>> Looks generally good to me, a few minor cosmetic comments below.
>>
>>>  libavcodec/Makefile  |   2 +
>>>  libavcodec/avcodec.h |  13 
>>>  libavcodec/encryption_info.c | 139 
>>> +++
>>>  libavcodec/encryption_info.h | 121 +
>>>  4 files changed, 275 insertions(+)
>>>  create mode 100644 libavcodec/encryption_info.c
>>>  create mode 100644 libavcodec/encryption_info.h
>>>
>>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>>> index ab7893f560..11ad642c6c 100644
>>> --- a/libavcodec/Makefile
>>> +++ b/libavcodec/Makefile
>>> @@ -10,6 +10,7 @@ HEADERS = ac3_parser.h
>>>   \
>>>dirac.h   \
>>>dv_profile.h  \
>>>dxva2.h   \
>>> +  encryption_info.h \
>>>jni.h \
>>>mediacodec.h  \
>>>qsv.h \
>>> @@ -36,6 +37,7 @@ OBJS = ac3_parser.o   
>>>   \
>>> dirac.o  \
>>> dv_profile.o \
>>> encode.o \
>>> +   encryption_info.o\
>>> imgconvert.o \
>>> jni.o\
>>> mathtables.o \
>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>> index 5db6a81320..b43638ebc5 100644
>>> --- a/libavcodec/avcodec.h
>>> +++ b/libavcodec/avcodec.h
>>> @@ -1327,6 +1327,19 @@ enum AVPacketSideDataType {
>>>   */
>>>  AV_PKT_DATA_A53_CC,
>>>
>>> +/**
>>> + * This side data is encryption "initialization data".
>>> + * For MP4 this is the entire 'pssh' box.
>>> + * For WebM this is the key ID.
>>> + */
>>> +AV_PKT_DATA_ENCRYPTION_INIT_DATA,
>>> +
>>> +/**
>>> + * This side data contains encryption info for how to decrypt the 
>>> packet.
>>> + * The format is not part of ABI, use av_encryption_info_* methods to 
>>> access.
>>> + */
>>> +AV_PKT_DATA_ENCRYPTION_INFO,
>>> +
>>>  /**
>>>   * The number of side data types.
>>>   * This is not part of the public API/ABI in the sense that it may
>>> diff --git a/libavcodec/encryption_info.c b/libavcodec/encryption_info.c
>>> new file mode 100644
>>> index 00..59ee4c41a9
>>> --- /dev/null
>>> +++ b/libavcodec/encryption_info.c
>>> @@ -0,0 +1,139 @@
>>> +/**
>>> + * This file is part of FFmpeg.
>>> + *
>>> + * FFmpeg is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2.1 of the License, or (at your option) any later version.
>>> + *
>>> + * FFmpeg is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with FFmpeg; if not, write to the Free Software
>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
>>> 02110-1301 USA
>>> + */
>>> +
>>> +#include "encryption_info.h"
>>> +#include "libavutil/avassert.h"
>>> +#include "libavutil/intreadwrite.h"
>>> +
>>> +#define FF_ENCRYPTION_INFO_EXTRA 24
>>> +
>>> +// The format of the side data:
>>> +// u32be scheme
>>> +// u32be crypt_byte_block
>>> +// u32be skip_byte_block
>>> +// u32be key_id_size
>>> +// u32be iv_size
>>> +// u32be subsample_count
>>> +// u8[key_id_size] key_id
>>> +// u8[iv_size] iv
>>> +// {

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Jacob Trimble]

On Wed, Dec 20, 2017 at 4:31 PM, wm4  wrote:
> On Wed, 20 Dec 2017 15:10:43 -0800
> Jacob Trimble  wrote:
>
>> From 1508d19e9f7acf43d76010ce54d59ff204613601 Mon Sep 17 00:00:00 2001
>> From: Jacob Trimble 
>> Date: Tue, 5 Dec 2017 14:52:22 -0800
>> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
>>
>> This new side-data will contain info on how a packet is encrypted.
>> This allows the app to handle packet decryption.
>>
>> Signed-off-by: Jacob Trimble 
>> ---
>
> Looks generally good to me, a few minor cosmetic comments below.
>
>>  libavcodec/Makefile  |   2 +
>>  libavcodec/avcodec.h |  13 
>>  libavcodec/encryption_info.c | 139 
>> +++
>>  libavcodec/encryption_info.h | 121 +
>>  4 files changed, 275 insertions(+)
>>  create mode 100644 libavcodec/encryption_info.c
>>  create mode 100644 libavcodec/encryption_info.h
>>
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index ab7893f560..11ad642c6c 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -10,6 +10,7 @@ HEADERS = ac3_parser.h 
>>  \
>>dirac.h   \
>>dv_profile.h  \
>>dxva2.h   \
>> +  encryption_info.h \
>>jni.h \
>>mediacodec.h  \
>>qsv.h \
>> @@ -36,6 +37,7 @@ OBJS = ac3_parser.o
>>  \
>> dirac.o  \
>> dv_profile.o \
>> encode.o \
>> +   encryption_info.o\
>> imgconvert.o \
>> jni.o\
>> mathtables.o \
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index 5db6a81320..b43638ebc5 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -1327,6 +1327,19 @@ enum AVPacketSideDataType {
>>   */
>>  AV_PKT_DATA_A53_CC,
>>
>> +/**
>> + * This side data is encryption "initialization data".
>> + * For MP4 this is the entire 'pssh' box.
>> + * For WebM this is the key ID.
>> + */
>> +AV_PKT_DATA_ENCRYPTION_INIT_DATA,
>> +
>> +/**
>> + * This side data contains encryption info for how to decrypt the 
>> packet.
>> + * The format is not part of ABI, use av_encryption_info_* methods to 
>> access.
>> + */
>> +AV_PKT_DATA_ENCRYPTION_INFO,
>> +
>>  /**
>>   * The number of side data types.
>>   * This is not part of the public API/ABI in the sense that it may
>> diff --git a/libavcodec/encryption_info.c b/libavcodec/encryption_info.c
>> new file mode 100644
>> index 00..59ee4c41a9
>> --- /dev/null
>> +++ b/libavcodec/encryption_info.c
>> @@ -0,0 +1,139 @@
>> +/**
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
>> USA
>> + */
>> +
>> +#include "encryption_info.h"
>> +#include "libavutil/avassert.h"
>> +#include "libavutil/intreadwrite.h"
>> +
>> +#define FF_ENCRYPTION_INFO_EXTRA 24
>> +
>> +// The format of the side data:
>> +// u32be scheme
>> +// u32be crypt_byte_block
>> +// u32be skip_byte_block
>> +// u32be key_id_size
>> +// u32be iv_size
>> +// u32be subsample_count
>> +// u8[key_id_size] key_id
>> +// u8[iv_size] iv
>> +// {
>> +//   u32be bytes_of_clear_data
>> +//   u32be bytes_of_protected_data
>> +// }[subsample_count]
>> +
>> +AVEncryptionInfo* av_encryption_info_alloc(uint32_t subsample_count, 
>> uint32_t 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[wm4]

On Wed, 20 Dec 2017 15:10:43 -0800
Jacob Trimble  wrote:

> From 1508d19e9f7acf43d76010ce54d59ff204613601 Mon Sep 17 00:00:00 2001
> From: Jacob Trimble 
> Date: Tue, 5 Dec 2017 14:52:22 -0800
> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
> 
> This new side-data will contain info on how a packet is encrypted.
> This allows the app to handle packet decryption.
> 
> Signed-off-by: Jacob Trimble 
> ---

Looks generally good to me, a few minor cosmetic comments below.

>  libavcodec/Makefile  |   2 +
>  libavcodec/avcodec.h |  13 
>  libavcodec/encryption_info.c | 139 
> +++
>  libavcodec/encryption_info.h | 121 +
>  4 files changed, 275 insertions(+)
>  create mode 100644 libavcodec/encryption_info.c
>  create mode 100644 libavcodec/encryption_info.h
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index ab7893f560..11ad642c6c 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -10,6 +10,7 @@ HEADERS = ac3_parser.h  
> \
>dirac.h   \
>dv_profile.h  \
>dxva2.h   \
> +  encryption_info.h \
>jni.h \
>mediacodec.h  \
>qsv.h \
> @@ -36,6 +37,7 @@ OBJS = ac3_parser.o 
> \
> dirac.o  \
> dv_profile.o \
> encode.o \
> +   encryption_info.o\
> imgconvert.o \
> jni.o\
> mathtables.o \
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 5db6a81320..b43638ebc5 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -1327,6 +1327,19 @@ enum AVPacketSideDataType {
>   */
>  AV_PKT_DATA_A53_CC,
>  
> +/**
> + * This side data is encryption "initialization data".
> + * For MP4 this is the entire 'pssh' box.
> + * For WebM this is the key ID.
> + */
> +AV_PKT_DATA_ENCRYPTION_INIT_DATA,
> +
> +/**
> + * This side data contains encryption info for how to decrypt the packet.
> + * The format is not part of ABI, use av_encryption_info_* methods to 
> access.
> + */
> +AV_PKT_DATA_ENCRYPTION_INFO,
> +
>  /**
>   * The number of side data types.
>   * This is not part of the public API/ABI in the sense that it may
> diff --git a/libavcodec/encryption_info.c b/libavcodec/encryption_info.c
> new file mode 100644
> index 00..59ee4c41a9
> --- /dev/null
> +++ b/libavcodec/encryption_info.c
> @@ -0,0 +1,139 @@
> +/**
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +
> +#include "encryption_info.h"
> +#include "libavutil/avassert.h"
> +#include "libavutil/intreadwrite.h"
> +
> +#define FF_ENCRYPTION_INFO_EXTRA 24
> +
> +// The format of the side data:
> +// u32be scheme
> +// u32be crypt_byte_block
> +// u32be skip_byte_block
> +// u32be key_id_size
> +// u32be iv_size
> +// u32be subsample_count
> +// u8[key_id_size] key_id
> +// u8[iv_size] iv
> +// {
> +//   u32be bytes_of_clear_data
> +//   u32be bytes_of_protected_data
> +// }[subsample_count]
> +
> +AVEncryptionInfo* av_encryption_info_alloc(uint32_t subsample_count, 
> uint32_t key_id_size, uint32_t iv_size)

I think we generally put the "*" to the name, to reflect the actual C
syntax. (Here and in other places where pointers are in the function
return or argument 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Jacob Trimble]

On Wed, Dec 20, 2017 at 12:23 PM, wm4  wrote:
> On Wed, 20 Dec 2017 12:07:09 -0800
> Jacob Trimble  wrote:
>
>> On Tue, Dec 19, 2017 at 3:05 PM, wm4  wrote:
>> > On Tue, 19 Dec 2017 14:20:38 -0800
>> > Jacob Trimble  wrote:
>> >
>> >> > I don't think this is sane. So far, side data could simply be copied
>> >> > with memcpy, and having pointers to non-static data in side data would
>> >> > break this completely
>> >>
>> >> Then how about storing the data like it is now (the encryption info
>> >> followed by the subsample array), but not have a pointer?  Then there
>> >> will be several helper methods to get the subsample array from the
>> >> side-data.  For example,
>> >> av_encryption_info_get_subsamples(AVPacketEncryptionInfo*) or
>> >> av_encryption_info_get_subsamples(AVPacket*) (since there will only be
>> >> one instance of it).
>> >
>> > I guess that would work? Not particularly fond of the idea anyway. I
>> > think the functions would probably work on the side data byte array,
>> > maybe.
>>
>> I'm not fond of this either, but I can't think of a way to allow a
>> dynamic number of elements while supporting memcpy and not requiring
>> the app to "parse" the side-data.
>>
>> So here is may latest attempt.  This has a binary format inside the
>> side-data that allows memcpy to work, but there is a public struct
>> that apps will interact with.  There are two methods used to convert
>> between the two so the app doesn't have to.  Even though this is a
>> binary format, it is not actually a wire format since the subsamples
>> are stored as-is, so they are host byte ordered.  Also, as Michael
>> requested, this uses dynamic sized key ID and IV.
>
> I think your new patch idea is pretty sane. I'd explicitly document
> that the encryption side data format is not part of the ABI, and that
> applications must access it with the av_encryption_info_*() functions.
>
> Also I think it would be better to avoid the memory allocation
> messiness by copying all data, instead of implicitly referencing the
> AVPacket data. Also provide a free function. (That is assuming you're
> fine with a copy - creating AVPacket references copies all side data
> anyway, so the whole code base assumes side data copies are cheap.)
>
> If AVEncryptionInfo ever needs to be extended, it would also be better
> to just let av_encryption_info_get() return a newly allocated struct,
> and to exclude the size of the struct from the ABI. Then adding new
> fields would not require waiting for an ABI bump.

Done
From 1508d19e9f7acf43d76010ce54d59ff204613601 Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
Date: Tue, 5 Dec 2017 14:52:22 -0800
Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.

This new side-data will contain info on how a packet is encrypted.
This allows the app to handle packet decryption.

Signed-off-by: Jacob Trimble 
---
 libavcodec/Makefile  |   2 +
 libavcodec/avcodec.h |  13 
 libavcodec/encryption_info.c | 139 +++
 libavcodec/encryption_info.h | 121 +
 4 files changed, 275 insertions(+)
 create mode 100644 libavcodec/encryption_info.c
 create mode 100644 libavcodec/encryption_info.h

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index ab7893f560..11ad642c6c 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -10,6 +10,7 @@ HEADERS = ac3_parser.h  \
   dirac.h   \
   dv_profile.h  \
   dxva2.h   \
+  encryption_info.h \
   jni.h \
   mediacodec.h  \
   qsv.h \
@@ -36,6 +37,7 @@ OBJS = ac3_parser.o \
dirac.o  \
dv_profile.o \
encode.o \
+   encryption_info.o\
imgconvert.o \
jni.o\
mathtables.o \
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 5db6a81320..b43638ebc5 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1327,6 +1327,19 @@ enum AVPacketSideDataType {
  */
 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[wm4]

On Wed, 20 Dec 2017 12:07:09 -0800
Jacob Trimble  wrote:

> On Tue, Dec 19, 2017 at 3:05 PM, wm4  wrote:
> > On Tue, 19 Dec 2017 14:20:38 -0800
> > Jacob Trimble  wrote:
> >
> >> > I don't think this is sane. So far, side data could simply be copied
> >> > with memcpy, and having pointers to non-static data in side data would
> >> > break this completely
> >>
> >> Then how about storing the data like it is now (the encryption info
> >> followed by the subsample array), but not have a pointer?  Then there
> >> will be several helper methods to get the subsample array from the
> >> side-data.  For example,
> >> av_encryption_info_get_subsamples(AVPacketEncryptionInfo*) or
> >> av_encryption_info_get_subsamples(AVPacket*) (since there will only be
> >> one instance of it).
> >
> > I guess that would work? Not particularly fond of the idea anyway. I
> > think the functions would probably work on the side data byte array,
> > maybe.
> 
> I'm not fond of this either, but I can't think of a way to allow a
> dynamic number of elements while supporting memcpy and not requiring
> the app to "parse" the side-data.
> 
> So here is may latest attempt.  This has a binary format inside the
> side-data that allows memcpy to work, but there is a public struct
> that apps will interact with.  There are two methods used to convert
> between the two so the app doesn't have to.  Even though this is a
> binary format, it is not actually a wire format since the subsamples
> are stored as-is, so they are host byte ordered.  Also, as Michael
> requested, this uses dynamic sized key ID and IV.

I think your new patch idea is pretty sane. I'd explicitly document
that the encryption side data format is not part of the ABI, and that
applications must access it with the av_encryption_info_*() functions.

Also I think it would be better to avoid the memory allocation
messiness by copying all data, instead of implicitly referencing the
AVPacket data. Also provide a free function. (That is assuming you're
fine with a copy - creating AVPacket references copies all side data
anyway, so the whole code base assumes side data copies are cheap.)

If AVEncryptionInfo ever needs to be extended, it would also be better
to just let av_encryption_info_get() return a newly allocated struct,
and to exclude the size of the struct from the ABI. Then adding new
fields would not require waiting for an ABI bump.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Jacob Trimble]

On Tue, Dec 19, 2017 at 3:05 PM, wm4  wrote:
> On Tue, 19 Dec 2017 14:20:38 -0800
> Jacob Trimble  wrote:
>
>> > I don't think this is sane. So far, side data could simply be copied
>> > with memcpy, and having pointers to non-static data in side data would
>> > break this completely
>>
>> Then how about storing the data like it is now (the encryption info
>> followed by the subsample array), but not have a pointer?  Then there
>> will be several helper methods to get the subsample array from the
>> side-data.  For example,
>> av_encryption_info_get_subsamples(AVPacketEncryptionInfo*) or
>> av_encryption_info_get_subsamples(AVPacket*) (since there will only be
>> one instance of it).
>
> I guess that would work? Not particularly fond of the idea anyway. I
> think the functions would probably work on the side data byte array,
> maybe.

I'm not fond of this either, but I can't think of a way to allow a
dynamic number of elements while supporting memcpy and not requiring
the app to "parse" the side-data.

So here is may latest attempt.  This has a binary format inside the
side-data that allows memcpy to work, but there is a public struct
that apps will interact with.  There are two methods used to convert
between the two so the app doesn't have to.  Even though this is a
binary format, it is not actually a wire format since the subsamples
are stored as-is, so they are host byte ordered.  Also, as Michael
requested, this uses dynamic sized key ID and IV.
From 38c480a470d7f107945180968a5db237c671f7d0 Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
Date: Tue, 5 Dec 2017 14:52:22 -0800
Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.

This new side-data will contain info on how a packet is encrypted.
This allows the app to handle packet decryption.

Signed-off-by: Jacob Trimble 
---
 libavcodec/Makefile  |   2 +
 libavcodec/avcodec.h |  13 ++
 libavcodec/encryption_info.c |  70 +
 libavcodec/encryption_info.h | 105 +++
 4 files changed, 190 insertions(+)
 create mode 100644 libavcodec/encryption_info.c
 create mode 100644 libavcodec/encryption_info.h

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index ab7893f560..11ad642c6c 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -10,6 +10,7 @@ HEADERS = ac3_parser.h  \
   dirac.h   \
   dv_profile.h  \
   dxva2.h   \
+  encryption_info.h \
   jni.h \
   mediacodec.h  \
   qsv.h \
@@ -36,6 +37,7 @@ OBJS = ac3_parser.o \
dirac.o  \
dv_profile.o \
encode.o \
+   encryption_info.o\
imgconvert.o \
jni.o\
mathtables.o \
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 5db6a81320..7a5bba2687 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1327,6 +1327,19 @@ enum AVPacketSideDataType {
  */
 AV_PKT_DATA_A53_CC,
 
+/**
+ * This side data is encryption "initialization data".
+ * For MP4 this is the entire 'pssh' box.
+ * For WebM this is the key ID.
+ */
+AV_PKT_DATA_ENCRYPTION_INIT_DATA,
+
+/**
+ * This side data contains encryption info for how to decrypt the packet.
+ * Use av_encryption_info_get to get the info out of this side data.
+ */
+AV_PKT_DATA_ENCRYPTION_INFO,
+
 /**
  * The number of side data types.
  * This is not part of the public API/ABI in the sense that it may
diff --git a/libavcodec/encryption_info.c b/libavcodec/encryption_info.c
new file mode 100644
index 00..a2c11fe213
--- /dev/null
+++ b/libavcodec/encryption_info.c
@@ -0,0 +1,70 @@
+/**
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[wm4]

On Tue, 19 Dec 2017 14:20:38 -0800
Jacob Trimble  wrote:

> > You pretty much did. Side data is an ffmpeg internal concept, and if
> > your hypothetical streaming protocol needs special support for side
> > data, it's automatically relying on ffmpeg internals.  
> 
> I thought side data was public data?  Doesn't it contain public info
> like display info and required decoder info?  So if someone wants to
> decode frames without using the demuxers, they'll need to reconstruct
> the side data so the decoder can still work.

Side data is ffmpeg API, but it's a ffmpeg specific concept, and
tightly bound to its internal workings. Using it for a streaming
protocol would essentially mean both client and server have to be
ffmpeg. That doesn't make for a good (standardizable) protocol.

> > On that note, I wonder if we should add an AVPacket reserved field for
> > this, just so we don't have to wait for the next ABI bump.  
> 
> I would much prefer to have this in AVPacket since it is in no way
> "optional".  Having the info in the packet ensures a custom app can
> see the info and is less likely to get confused on why their packets
> aren't decoding.  I did this as side-data since it would have less ABI
> impact and I thought it would cause less conflict.  But having it as
> another field on AVPacket would allow more control over memory so the
> dynamic fields don't cause problems anymore.

I was talking about making a new side data API, which could handle
cases like this much better, instead of just being byte arrays. I don't
think we should add anything specific to encryption directly to
AVPacket, it's an too obscure use case. (The whole point of side data
was to avoid adding new fields for obscure use cases to top level
structs. Just look how many weird fields AVCodecContext has.)

> > I don't think this is sane. So far, side data could simply be copied
> > with memcpy, and having pointers to non-static data in side data would
> > break this completely  
> 
> Then how about storing the data like it is now (the encryption info
> followed by the subsample array), but not have a pointer?  Then there
> will be several helper methods to get the subsample array from the
> side-data.  For example,
> av_encryption_info_get_subsamples(AVPacketEncryptionInfo*) or
> av_encryption_info_get_subsamples(AVPacket*) (since there will only be
> one instance of it).

I guess that would work? Not particularly fond of the idea anyway. I
think the functions would probably work on the side data byte array,
maybe.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Jacob Trimble]

> You pretty much did. Side data is an ffmpeg internal concept, and if
> your hypothetical streaming protocol needs special support for side
> data, it's automatically relying on ffmpeg internals.

I thought side data was public data?  Doesn't it contain public info
like display info and required decoder info?  So if someone wants to
decode frames without using the demuxers, they'll need to reconstruct
the side data so the decoder can still work.

> On that note, I wonder if we should add an AVPacket reserved field for
> this, just so we don't have to wait for the next ABI bump.

I would much prefer to have this in AVPacket since it is in no way
"optional".  Having the info in the packet ensures a custom app can
see the info and is less likely to get confused on why their packets
aren't decoding.  I did this as side-data since it would have less ABI
impact and I thought it would cause less conflict.  But having it as
another field on AVPacket would allow more control over memory so the
dynamic fields don't cause problems anymore.

> I don't think this is sane. So far, side data could simply be copied
> with memcpy, and having pointers to non-static data in side data would
> break this completely

Then how about storing the data like it is now (the encryption info
followed by the subsample array), but not have a pointer?  Then there
will be several helper methods to get the subsample array from the
side-data.  For example,
av_encryption_info_get_subsamples(AVPacketEncryptionInfo*) or
av_encryption_info_get_subsamples(AVPacket*) (since there will only be
one instance of it).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Michael Niedermayer]

On Mon, Dec 18, 2017 at 06:29:14PM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Mon, Dec 18, 2017 at 6:00 PM, Michael Niedermayer  > wrote:
> 
> > If you decode this again you need the side data in the format of the
> > platform
> > the decoder runs on. This is very very basic logic. Something must convert
> > it
> 
> 
> Right, this concept is typically called a "serializer" and "deserializer".
> Adding them to the API is a good idea. But I don't understand why we'd want
> to store side-data as binary (platform-independent) data, that would ruin
> it accessibility from applications. Applications don't want to call hton or
> ntoh on each structure member they access, right?

yes, i fully agree

-- 
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] [RFC] avcodec/avcodec.h: Add encryption info side data

[wm4]

On Tue, 19 Dec 2017 01:14:37 +0100
Michael Niedermayer  wrote:

> On Tue, Dec 19, 2017 at 12:17:11AM +0100, wm4 wrote:
> > On Tue, 19 Dec 2017 00:00:15 +0100
> > Michael Niedermayer  wrote:
> >   
> > > On Mon, Dec 18, 2017 at 10:28:14PM +0100, wm4 wrote:  
> > > > On Mon, 18 Dec 2017 22:17:14 +0100
> > > > Michael Niedermayer  wrote:
> > > > 
> > > > > On Mon, Dec 18, 2017 at 10:02:52PM +0100, wm4 wrote:
> > > > > > On Mon, 18 Dec 2017 21:38:24 +0100
> > > > > > Michael Niedermayer  wrote:
> > > > > >   
> > > > > > > On Mon, Dec 18, 2017 at 04:56:08PM -0300, James Almer wrote:  
> > > > > > > > On 12/18/2017 4:52 PM, wm4 wrote:
> > > > > > > > > On Fri, 15 Dec 2017 14:24:17 -0800
> > > > > > > > > Jacob Trimble  wrote:
> > > > > > > > > 
> > > > > > > > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 
> > > > > > > > >> 00:00:00 2001
> > > > > > > > >> From: Jacob Trimble 
> > > > > > > > >> Date: Tue, 5 Dec 2017 14:52:22 -0800
> > > > > > > > >> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side 
> > > > > > > > >> data.
> > > > > > > > >>
> > > > > > > > >> This new side-data will contain info on how a packet is 
> > > > > > > > >> encrypted.
> > > > > > > > >> This allows the app to handle packet decryption.  To allow 
> > > > > > > > >> for a
> > > > > > > > >> variable number of subsamples, the buffer for the side-data 
> > > > > > > > >> will be
> > > > > > > > >> allocated to hold both the structure and the array of 
> > > > > > > > >> subsamples.  So
> > > > > > > > >> the |subsamples| member will point to right after the struct.
> > > > > > > > >>
> > > > > > > > >> Signed-off-by: Jacob Trimble 
> > > > > > > > >> ---
> > > > > > > > >>  libavcodec/avcodec.h | 70 
> > > > > > > > >> 
> > > > > > > > >>  1 file changed, 70 insertions(+)
> > > > > > > > >>
> > > > > > > > >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > > > > > >> index 5db6a81320..ccc89345e8 100644
> > > > > > > > >> --- a/libavcodec/avcodec.h
> > > > > > > > >> +++ b/libavcodec/avcodec.h
> > > > > > > > >> @@ -1112,6 +1112,63 @@ typedef struct AVCPBProperties {
> > > > > > > > >>  uint64_t vbv_delay;
> > > > > > > > >>  } AVCPBProperties;
> > > > > > > > >>  
> > > > > > > > >> +typedef struct AVPacketSubsampleEncryptionInfo {
> > > > > > > > >> +/** The number of bytes that are clear. */
> > > > > > > > >> +unsigned int bytes_of_clear_data;
> > > > > > > > >> +
> > > > > > > > >> +/**
> > > > > > > > >> + * The number of bytes that are protected.  If using 
> > > > > > > > >> pattern encryption,
> > > > > > > > >> + * the pattern applies to only the protected bytes; if 
> > > > > > > > >> not using pattern
> > > > > > > > >> + * encryption, all these bytes are encrypted.
> > > > > > > > >> + */
> > > > > > > > >> +unsigned int bytes_of_protected_data;
> > > > > > > > >> +} AVPacketSubsampleEncryptionInfo;
> > > > > > > > >> +
> > > > > > > > >> +/**
> > > > > > > > >> + * This describes encryption info for a packet.  This 
> > > > > > > > >> contains frame-specific
> > > > > > > > >> + * info for how to decrypt the packet before passing it to 
> > > > > > > > >> the decoder.  If this
> > > > > > > > >> + * side-data is present, then the packet is encrypted.
> > > > > > > > >> + */
> > > > > > > > >> +typedef struct AVPacketEncryptionInfo {
> > > > > > > > >> +/** The fourcc encryption scheme. */
> > > > > > > > >> +uint32_t scheme;
> > > > > > > > >> +
> > > > > > > > >> +/** The ID of the key used to encrypt the packet. */
> > > > > > > > >> +uint8_t key_id[16];
> > > > > > > > >> +
> > > > > > > > >> +/** The initialization vector. */
> > > > > > > > >> +uint8_t iv[16];
> > > > > > > > >> +
> > > > > > > > >> +/**
> > > > > > > > >> + * Only used for pattern encryption.  This is the 
> > > > > > > > >> number of 16-byte blocks
> > > > > > > > >> + * that are encrypted.
> > > > > > > > >> + */
> > > > > > > > >> +unsigned int crypt_byte_block;
> > > > > > > > >> +
> > > > > > > > >> +/**
> > > > > > > > >> + * Only used for pattern encryption.  This is the 
> > > > > > > > >> number of 16-byte blocks
> > > > > > > > >> + * that are clear.
> > > > > > > > >> + */
> > > > > > > > >> +unsigned int skip_byte_block;
> > > > > > > > >> +
> > > > > > > > >> +/**
> > > > > > > > >> + * The number of sub-samples in this packet.  If 0, 
> > > > > > > > >> then the whole sample
> > > > > > > > >> + * is encrypted.
> > > > > > > > >> + */
> > > > > > > > >> +unsigned int subsample_count;
> > > > > > > > >> +
> > > > > > > > >> +/** The subsample encryption info. */
> > > > > > > > >> +AVPacketSubsampleEncryptionInfo 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Michael Niedermayer]

On Tue, Dec 19, 2017 at 12:17:11AM +0100, wm4 wrote:
> On Tue, 19 Dec 2017 00:00:15 +0100
> Michael Niedermayer  wrote:
> 
> > On Mon, Dec 18, 2017 at 10:28:14PM +0100, wm4 wrote:
> > > On Mon, 18 Dec 2017 22:17:14 +0100
> > > Michael Niedermayer  wrote:
> > >   
> > > > On Mon, Dec 18, 2017 at 10:02:52PM +0100, wm4 wrote:  
> > > > > On Mon, 18 Dec 2017 21:38:24 +0100
> > > > > Michael Niedermayer  wrote:
> > > > > 
> > > > > > On Mon, Dec 18, 2017 at 04:56:08PM -0300, James Almer wrote:
> > > > > > > On 12/18/2017 4:52 PM, wm4 wrote:  
> > > > > > > > On Fri, 15 Dec 2017 14:24:17 -0800
> > > > > > > > Jacob Trimble  wrote:
> > > > > > > >   
> > > > > > > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 
> > > > > > > >> 00:00:00 2001
> > > > > > > >> From: Jacob Trimble 
> > > > > > > >> Date: Tue, 5 Dec 2017 14:52:22 -0800
> > > > > > > >> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side 
> > > > > > > >> data.
> > > > > > > >>
> > > > > > > >> This new side-data will contain info on how a packet is 
> > > > > > > >> encrypted.
> > > > > > > >> This allows the app to handle packet decryption.  To allow for 
> > > > > > > >> a
> > > > > > > >> variable number of subsamples, the buffer for the side-data 
> > > > > > > >> will be
> > > > > > > >> allocated to hold both the structure and the array of 
> > > > > > > >> subsamples.  So
> > > > > > > >> the |subsamples| member will point to right after the struct.
> > > > > > > >>
> > > > > > > >> Signed-off-by: Jacob Trimble 
> > > > > > > >> ---
> > > > > > > >>  libavcodec/avcodec.h | 70 
> > > > > > > >> 
> > > > > > > >>  1 file changed, 70 insertions(+)
> > > > > > > >>
> > > > > > > >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > > > > >> index 5db6a81320..ccc89345e8 100644
> > > > > > > >> --- a/libavcodec/avcodec.h
> > > > > > > >> +++ b/libavcodec/avcodec.h
> > > > > > > >> @@ -1112,6 +1112,63 @@ typedef struct AVCPBProperties {
> > > > > > > >>  uint64_t vbv_delay;
> > > > > > > >>  } AVCPBProperties;
> > > > > > > >>  
> > > > > > > >> +typedef struct AVPacketSubsampleEncryptionInfo {
> > > > > > > >> +/** The number of bytes that are clear. */
> > > > > > > >> +unsigned int bytes_of_clear_data;
> > > > > > > >> +
> > > > > > > >> +/**
> > > > > > > >> + * The number of bytes that are protected.  If using 
> > > > > > > >> pattern encryption,
> > > > > > > >> + * the pattern applies to only the protected bytes; if 
> > > > > > > >> not using pattern
> > > > > > > >> + * encryption, all these bytes are encrypted.
> > > > > > > >> + */
> > > > > > > >> +unsigned int bytes_of_protected_data;
> > > > > > > >> +} AVPacketSubsampleEncryptionInfo;
> > > > > > > >> +
> > > > > > > >> +/**
> > > > > > > >> + * This describes encryption info for a packet.  This 
> > > > > > > >> contains frame-specific
> > > > > > > >> + * info for how to decrypt the packet before passing it to 
> > > > > > > >> the decoder.  If this
> > > > > > > >> + * side-data is present, then the packet is encrypted.
> > > > > > > >> + */
> > > > > > > >> +typedef struct AVPacketEncryptionInfo {
> > > > > > > >> +/** The fourcc encryption scheme. */
> > > > > > > >> +uint32_t scheme;
> > > > > > > >> +
> > > > > > > >> +/** The ID of the key used to encrypt the packet. */
> > > > > > > >> +uint8_t key_id[16];
> > > > > > > >> +
> > > > > > > >> +/** The initialization vector. */
> > > > > > > >> +uint8_t iv[16];
> > > > > > > >> +
> > > > > > > >> +/**
> > > > > > > >> + * Only used for pattern encryption.  This is the number 
> > > > > > > >> of 16-byte blocks
> > > > > > > >> + * that are encrypted.
> > > > > > > >> + */
> > > > > > > >> +unsigned int crypt_byte_block;
> > > > > > > >> +
> > > > > > > >> +/**
> > > > > > > >> + * Only used for pattern encryption.  This is the number 
> > > > > > > >> of 16-byte blocks
> > > > > > > >> + * that are clear.
> > > > > > > >> + */
> > > > > > > >> +unsigned int skip_byte_block;
> > > > > > > >> +
> > > > > > > >> +/**
> > > > > > > >> + * The number of sub-samples in this packet.  If 0, then 
> > > > > > > >> the whole sample
> > > > > > > >> + * is encrypted.
> > > > > > > >> + */
> > > > > > > >> +unsigned int subsample_count;
> > > > > > > >> +
> > > > > > > >> +/** The subsample encryption info. */
> > > > > > > >> +AVPacketSubsampleEncryptionInfo *subsamples;  
> > > > > > > > 
> > > > > > > > I don't think this is sane. So far, side data could simply be 
> > > > > > > > copied
> > > > > > > > with memcpy, and having pointers to non-static data in side 
> > > > > > > > data would
> > > > > > > > break this completely.  
> > > > > > > 
> > > > > 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[wm4]

On Tue, 19 Dec 2017 00:36:36 +0100
Carl Eugen Hoyos  wrote:

> 2017-12-19 0:17 GMT+01:00 wm4 :
> 
> [...]
> 
> Instead of sending random insults, could you just go away?

Why don't you follow your own advice? At this point you're acting like
a miserable troll who has absolutely nothing to contribute.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Carl Eugen Hoyos]

2017-12-19 0:17 GMT+01:00 wm4 :

[...]

Instead of sending random insults, could you just go away?

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

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Ronald S. Bultje]

Hi,

On Mon, Dec 18, 2017 at 6:00 PM, Michael Niedermayer  wrote:

> If you decode this again you need the side data in the format of the
> platform
> the decoder runs on. This is very very basic logic. Something must convert
> it


Right, this concept is typically called a "serializer" and "deserializer".
Adding them to the API is a good idea. But I don't understand why we'd want
to store side-data as binary (platform-independent) data, that would ruin
it accessibility from applications. Applications don't want to call hton or
ntoh on each structure member they access, right?

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

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[wm4]

On Tue, 19 Dec 2017 00:00:15 +0100
Michael Niedermayer  wrote:

> On Mon, Dec 18, 2017 at 10:28:14PM +0100, wm4 wrote:
> > On Mon, 18 Dec 2017 22:17:14 +0100
> > Michael Niedermayer  wrote:
> >   
> > > On Mon, Dec 18, 2017 at 10:02:52PM +0100, wm4 wrote:  
> > > > On Mon, 18 Dec 2017 21:38:24 +0100
> > > > Michael Niedermayer  wrote:
> > > > 
> > > > > On Mon, Dec 18, 2017 at 04:56:08PM -0300, James Almer wrote:
> > > > > > On 12/18/2017 4:52 PM, wm4 wrote:  
> > > > > > > On Fri, 15 Dec 2017 14:24:17 -0800
> > > > > > > Jacob Trimble  wrote:
> > > > > > >   
> > > > > > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 
> > > > > > >> 00:00:00 2001
> > > > > > >> From: Jacob Trimble 
> > > > > > >> Date: Tue, 5 Dec 2017 14:52:22 -0800
> > > > > > >> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side 
> > > > > > >> data.
> > > > > > >>
> > > > > > >> This new side-data will contain info on how a packet is 
> > > > > > >> encrypted.
> > > > > > >> This allows the app to handle packet decryption.  To allow for a
> > > > > > >> variable number of subsamples, the buffer for the side-data will 
> > > > > > >> be
> > > > > > >> allocated to hold both the structure and the array of 
> > > > > > >> subsamples.  So
> > > > > > >> the |subsamples| member will point to right after the struct.
> > > > > > >>
> > > > > > >> Signed-off-by: Jacob Trimble 
> > > > > > >> ---
> > > > > > >>  libavcodec/avcodec.h | 70 
> > > > > > >> 
> > > > > > >>  1 file changed, 70 insertions(+)
> > > > > > >>
> > > > > > >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > > > >> index 5db6a81320..ccc89345e8 100644
> > > > > > >> --- a/libavcodec/avcodec.h
> > > > > > >> +++ b/libavcodec/avcodec.h
> > > > > > >> @@ -1112,6 +1112,63 @@ typedef struct AVCPBProperties {
> > > > > > >>  uint64_t vbv_delay;
> > > > > > >>  } AVCPBProperties;
> > > > > > >>  
> > > > > > >> +typedef struct AVPacketSubsampleEncryptionInfo {
> > > > > > >> +/** The number of bytes that are clear. */
> > > > > > >> +unsigned int bytes_of_clear_data;
> > > > > > >> +
> > > > > > >> +/**
> > > > > > >> + * The number of bytes that are protected.  If using 
> > > > > > >> pattern encryption,
> > > > > > >> + * the pattern applies to only the protected bytes; if not 
> > > > > > >> using pattern
> > > > > > >> + * encryption, all these bytes are encrypted.
> > > > > > >> + */
> > > > > > >> +unsigned int bytes_of_protected_data;
> > > > > > >> +} AVPacketSubsampleEncryptionInfo;
> > > > > > >> +
> > > > > > >> +/**
> > > > > > >> + * This describes encryption info for a packet.  This contains 
> > > > > > >> frame-specific
> > > > > > >> + * info for how to decrypt the packet before passing it to the 
> > > > > > >> decoder.  If this
> > > > > > >> + * side-data is present, then the packet is encrypted.
> > > > > > >> + */
> > > > > > >> +typedef struct AVPacketEncryptionInfo {
> > > > > > >> +/** The fourcc encryption scheme. */
> > > > > > >> +uint32_t scheme;
> > > > > > >> +
> > > > > > >> +/** The ID of the key used to encrypt the packet. */
> > > > > > >> +uint8_t key_id[16];
> > > > > > >> +
> > > > > > >> +/** The initialization vector. */
> > > > > > >> +uint8_t iv[16];
> > > > > > >> +
> > > > > > >> +/**
> > > > > > >> + * Only used for pattern encryption.  This is the number of 
> > > > > > >> 16-byte blocks
> > > > > > >> + * that are encrypted.
> > > > > > >> + */
> > > > > > >> +unsigned int crypt_byte_block;
> > > > > > >> +
> > > > > > >> +/**
> > > > > > >> + * Only used for pattern encryption.  This is the number of 
> > > > > > >> 16-byte blocks
> > > > > > >> + * that are clear.
> > > > > > >> + */
> > > > > > >> +unsigned int skip_byte_block;
> > > > > > >> +
> > > > > > >> +/**
> > > > > > >> + * The number of sub-samples in this packet.  If 0, then 
> > > > > > >> the whole sample
> > > > > > >> + * is encrypted.
> > > > > > >> + */
> > > > > > >> +unsigned int subsample_count;
> > > > > > >> +
> > > > > > >> +/** The subsample encryption info. */
> > > > > > >> +AVPacketSubsampleEncryptionInfo *subsamples;  
> > > > > > > 
> > > > > > > I don't think this is sane. So far, side data could simply be 
> > > > > > > copied
> > > > > > > with memcpy, and having pointers to non-static data in side data 
> > > > > > > would
> > > > > > > break this completely.  
> > > > > > 
> > > > > > Even more reasons to ditch the current side data API and come up 
> > > > > > with a
> > > > > > better designed one that can also be reused for packet, frame and
> > > > > > container needs.  
> > > > > 
> > > > > yes, 
> > > > > also if its redesigned, it should become 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Michael Niedermayer]

On Mon, Dec 18, 2017 at 10:28:14PM +0100, wm4 wrote:
> On Mon, 18 Dec 2017 22:17:14 +0100
> Michael Niedermayer  wrote:
> 
> > On Mon, Dec 18, 2017 at 10:02:52PM +0100, wm4 wrote:
> > > On Mon, 18 Dec 2017 21:38:24 +0100
> > > Michael Niedermayer  wrote:
> > >   
> > > > On Mon, Dec 18, 2017 at 04:56:08PM -0300, James Almer wrote:  
> > > > > On 12/18/2017 4:52 PM, wm4 wrote:
> > > > > > On Fri, 15 Dec 2017 14:24:17 -0800
> > > > > > Jacob Trimble  wrote:
> > > > > > 
> > > > > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 
> > > > > >> 2001
> > > > > >> From: Jacob Trimble 
> > > > > >> Date: Tue, 5 Dec 2017 14:52:22 -0800
> > > > > >> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
> > > > > >>
> > > > > >> This new side-data will contain info on how a packet is encrypted.
> > > > > >> This allows the app to handle packet decryption.  To allow for a
> > > > > >> variable number of subsamples, the buffer for the side-data will be
> > > > > >> allocated to hold both the structure and the array of subsamples.  
> > > > > >> So
> > > > > >> the |subsamples| member will point to right after the struct.
> > > > > >>
> > > > > >> Signed-off-by: Jacob Trimble 
> > > > > >> ---
> > > > > >>  libavcodec/avcodec.h | 70 
> > > > > >> 
> > > > > >>  1 file changed, 70 insertions(+)
> > > > > >>
> > > > > >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > > >> index 5db6a81320..ccc89345e8 100644
> > > > > >> --- a/libavcodec/avcodec.h
> > > > > >> +++ b/libavcodec/avcodec.h
> > > > > >> @@ -1112,6 +1112,63 @@ typedef struct AVCPBProperties {
> > > > > >>  uint64_t vbv_delay;
> > > > > >>  } AVCPBProperties;
> > > > > >>  
> > > > > >> +typedef struct AVPacketSubsampleEncryptionInfo {
> > > > > >> +/** The number of bytes that are clear. */
> > > > > >> +unsigned int bytes_of_clear_data;
> > > > > >> +
> > > > > >> +/**
> > > > > >> + * The number of bytes that are protected.  If using pattern 
> > > > > >> encryption,
> > > > > >> + * the pattern applies to only the protected bytes; if not 
> > > > > >> using pattern
> > > > > >> + * encryption, all these bytes are encrypted.
> > > > > >> + */
> > > > > >> +unsigned int bytes_of_protected_data;
> > > > > >> +} AVPacketSubsampleEncryptionInfo;
> > > > > >> +
> > > > > >> +/**
> > > > > >> + * This describes encryption info for a packet.  This contains 
> > > > > >> frame-specific
> > > > > >> + * info for how to decrypt the packet before passing it to the 
> > > > > >> decoder.  If this
> > > > > >> + * side-data is present, then the packet is encrypted.
> > > > > >> + */
> > > > > >> +typedef struct AVPacketEncryptionInfo {
> > > > > >> +/** The fourcc encryption scheme. */
> > > > > >> +uint32_t scheme;
> > > > > >> +
> > > > > >> +/** The ID of the key used to encrypt the packet. */
> > > > > >> +uint8_t key_id[16];
> > > > > >> +
> > > > > >> +/** The initialization vector. */
> > > > > >> +uint8_t iv[16];
> > > > > >> +
> > > > > >> +/**
> > > > > >> + * Only used for pattern encryption.  This is the number of 
> > > > > >> 16-byte blocks
> > > > > >> + * that are encrypted.
> > > > > >> + */
> > > > > >> +unsigned int crypt_byte_block;
> > > > > >> +
> > > > > >> +/**
> > > > > >> + * Only used for pattern encryption.  This is the number of 
> > > > > >> 16-byte blocks
> > > > > >> + * that are clear.
> > > > > >> + */
> > > > > >> +unsigned int skip_byte_block;
> > > > > >> +
> > > > > >> +/**
> > > > > >> + * The number of sub-samples in this packet.  If 0, then the 
> > > > > >> whole sample
> > > > > >> + * is encrypted.
> > > > > >> + */
> > > > > >> +unsigned int subsample_count;
> > > > > >> +
> > > > > >> +/** The subsample encryption info. */
> > > > > >> +AVPacketSubsampleEncryptionInfo *subsamples;
> > > > > > 
> > > > > > I don't think this is sane. So far, side data could simply be copied
> > > > > > with memcpy, and having pointers to non-static data in side data 
> > > > > > would
> > > > > > break this completely.
> > > > > 
> > > > > Even more reasons to ditch the current side data API and come up with 
> > > > > a
> > > > > better designed one that can also be reused for packet, frame and
> > > > > container needs.
> > > > 
> > > > yes, 
> > > > also if its redesigned, it should become possible to pass it over the
> > > > network. Currently all side data depends on the platform ABI, so its
> > > > only valid on the platform its created on. That also makes it different
> > > > from all other data, AVPacket.data and extradata are all
> > > > platform independant. And there is no API to
> > > > convert it into a platform ABI independant form.   
> > > 
> > > Why should 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[wm4]

On Mon, 18 Dec 2017 22:17:14 +0100
Michael Niedermayer  wrote:

> On Mon, Dec 18, 2017 at 10:02:52PM +0100, wm4 wrote:
> > On Mon, 18 Dec 2017 21:38:24 +0100
> > Michael Niedermayer  wrote:
> >   
> > > On Mon, Dec 18, 2017 at 04:56:08PM -0300, James Almer wrote:  
> > > > On 12/18/2017 4:52 PM, wm4 wrote:
> > > > > On Fri, 15 Dec 2017 14:24:17 -0800
> > > > > Jacob Trimble  wrote:
> > > > > 
> > > > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 
> > > > >> 2001
> > > > >> From: Jacob Trimble 
> > > > >> Date: Tue, 5 Dec 2017 14:52:22 -0800
> > > > >> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
> > > > >>
> > > > >> This new side-data will contain info on how a packet is encrypted.
> > > > >> This allows the app to handle packet decryption.  To allow for a
> > > > >> variable number of subsamples, the buffer for the side-data will be
> > > > >> allocated to hold both the structure and the array of subsamples.  So
> > > > >> the |subsamples| member will point to right after the struct.
> > > > >>
> > > > >> Signed-off-by: Jacob Trimble 
> > > > >> ---
> > > > >>  libavcodec/avcodec.h | 70 
> > > > >> 
> > > > >>  1 file changed, 70 insertions(+)
> > > > >>
> > > > >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > >> index 5db6a81320..ccc89345e8 100644
> > > > >> --- a/libavcodec/avcodec.h
> > > > >> +++ b/libavcodec/avcodec.h
> > > > >> @@ -1112,6 +1112,63 @@ typedef struct AVCPBProperties {
> > > > >>  uint64_t vbv_delay;
> > > > >>  } AVCPBProperties;
> > > > >>  
> > > > >> +typedef struct AVPacketSubsampleEncryptionInfo {
> > > > >> +/** The number of bytes that are clear. */
> > > > >> +unsigned int bytes_of_clear_data;
> > > > >> +
> > > > >> +/**
> > > > >> + * The number of bytes that are protected.  If using pattern 
> > > > >> encryption,
> > > > >> + * the pattern applies to only the protected bytes; if not 
> > > > >> using pattern
> > > > >> + * encryption, all these bytes are encrypted.
> > > > >> + */
> > > > >> +unsigned int bytes_of_protected_data;
> > > > >> +} AVPacketSubsampleEncryptionInfo;
> > > > >> +
> > > > >> +/**
> > > > >> + * This describes encryption info for a packet.  This contains 
> > > > >> frame-specific
> > > > >> + * info for how to decrypt the packet before passing it to the 
> > > > >> decoder.  If this
> > > > >> + * side-data is present, then the packet is encrypted.
> > > > >> + */
> > > > >> +typedef struct AVPacketEncryptionInfo {
> > > > >> +/** The fourcc encryption scheme. */
> > > > >> +uint32_t scheme;
> > > > >> +
> > > > >> +/** The ID of the key used to encrypt the packet. */
> > > > >> +uint8_t key_id[16];
> > > > >> +
> > > > >> +/** The initialization vector. */
> > > > >> +uint8_t iv[16];
> > > > >> +
> > > > >> +/**
> > > > >> + * Only used for pattern encryption.  This is the number of 
> > > > >> 16-byte blocks
> > > > >> + * that are encrypted.
> > > > >> + */
> > > > >> +unsigned int crypt_byte_block;
> > > > >> +
> > > > >> +/**
> > > > >> + * Only used for pattern encryption.  This is the number of 
> > > > >> 16-byte blocks
> > > > >> + * that are clear.
> > > > >> + */
> > > > >> +unsigned int skip_byte_block;
> > > > >> +
> > > > >> +/**
> > > > >> + * The number of sub-samples in this packet.  If 0, then the 
> > > > >> whole sample
> > > > >> + * is encrypted.
> > > > >> + */
> > > > >> +unsigned int subsample_count;
> > > > >> +
> > > > >> +/** The subsample encryption info. */
> > > > >> +AVPacketSubsampleEncryptionInfo *subsamples;
> > > > > 
> > > > > I don't think this is sane. So far, side data could simply be copied
> > > > > with memcpy, and having pointers to non-static data in side data would
> > > > > break this completely.
> > > > 
> > > > Even more reasons to ditch the current side data API and come up with a
> > > > better designed one that can also be reused for packet, frame and
> > > > container needs.
> > > 
> > > yes, 
> > > also if its redesigned, it should become possible to pass it over the
> > > network. Currently all side data depends on the platform ABI, so its
> > > only valid on the platform its created on. That also makes it different
> > > from all other data, AVPacket.data and extradata are all
> > > platform independant. And there is no API to
> > > convert it into a platform ABI independant form.   
> > 
> > Why should it be transferable over network? That makes no sense. We
> > don't have the ability to transfer AVFrame, AVPacket, or AVCodecContext
> > over network either.  
> 
> If you have an application that streams video, and on a different computer
> an application which plays that video you need to transfer the 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Michael Niedermayer]

On Mon, Dec 18, 2017 at 10:02:52PM +0100, wm4 wrote:
> On Mon, 18 Dec 2017 21:38:24 +0100
> Michael Niedermayer  wrote:
> 
> > On Mon, Dec 18, 2017 at 04:56:08PM -0300, James Almer wrote:
> > > On 12/18/2017 4:52 PM, wm4 wrote:  
> > > > On Fri, 15 Dec 2017 14:24:17 -0800
> > > > Jacob Trimble  wrote:
> > > >   
> > > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> > > >> From: Jacob Trimble 
> > > >> Date: Tue, 5 Dec 2017 14:52:22 -0800
> > > >> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
> > > >>
> > > >> This new side-data will contain info on how a packet is encrypted.
> > > >> This allows the app to handle packet decryption.  To allow for a
> > > >> variable number of subsamples, the buffer for the side-data will be
> > > >> allocated to hold both the structure and the array of subsamples.  So
> > > >> the |subsamples| member will point to right after the struct.
> > > >>
> > > >> Signed-off-by: Jacob Trimble 
> > > >> ---
> > > >>  libavcodec/avcodec.h | 70 
> > > >> 
> > > >>  1 file changed, 70 insertions(+)
> > > >>
> > > >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > >> index 5db6a81320..ccc89345e8 100644
> > > >> --- a/libavcodec/avcodec.h
> > > >> +++ b/libavcodec/avcodec.h
> > > >> @@ -1112,6 +1112,63 @@ typedef struct AVCPBProperties {
> > > >>  uint64_t vbv_delay;
> > > >>  } AVCPBProperties;
> > > >>  
> > > >> +typedef struct AVPacketSubsampleEncryptionInfo {
> > > >> +/** The number of bytes that are clear. */
> > > >> +unsigned int bytes_of_clear_data;
> > > >> +
> > > >> +/**
> > > >> + * The number of bytes that are protected.  If using pattern 
> > > >> encryption,
> > > >> + * the pattern applies to only the protected bytes; if not using 
> > > >> pattern
> > > >> + * encryption, all these bytes are encrypted.
> > > >> + */
> > > >> +unsigned int bytes_of_protected_data;
> > > >> +} AVPacketSubsampleEncryptionInfo;
> > > >> +
> > > >> +/**
> > > >> + * This describes encryption info for a packet.  This contains 
> > > >> frame-specific
> > > >> + * info for how to decrypt the packet before passing it to the 
> > > >> decoder.  If this
> > > >> + * side-data is present, then the packet is encrypted.
> > > >> + */
> > > >> +typedef struct AVPacketEncryptionInfo {
> > > >> +/** The fourcc encryption scheme. */
> > > >> +uint32_t scheme;
> > > >> +
> > > >> +/** The ID of the key used to encrypt the packet. */
> > > >> +uint8_t key_id[16];
> > > >> +
> > > >> +/** The initialization vector. */
> > > >> +uint8_t iv[16];
> > > >> +
> > > >> +/**
> > > >> + * Only used for pattern encryption.  This is the number of 
> > > >> 16-byte blocks
> > > >> + * that are encrypted.
> > > >> + */
> > > >> +unsigned int crypt_byte_block;
> > > >> +
> > > >> +/**
> > > >> + * Only used for pattern encryption.  This is the number of 
> > > >> 16-byte blocks
> > > >> + * that are clear.
> > > >> + */
> > > >> +unsigned int skip_byte_block;
> > > >> +
> > > >> +/**
> > > >> + * The number of sub-samples in this packet.  If 0, then the 
> > > >> whole sample
> > > >> + * is encrypted.
> > > >> + */
> > > >> +unsigned int subsample_count;
> > > >> +
> > > >> +/** The subsample encryption info. */
> > > >> +AVPacketSubsampleEncryptionInfo *subsamples;  
> > > > 
> > > > I don't think this is sane. So far, side data could simply be copied
> > > > with memcpy, and having pointers to non-static data in side data would
> > > > break this completely.  
> > > 
> > > Even more reasons to ditch the current side data API and come up with a
> > > better designed one that can also be reused for packet, frame and
> > > container needs.  
> > 
> > yes, 
> > also if its redesigned, it should become possible to pass it over the
> > network. Currently all side data depends on the platform ABI, so its
> > only valid on the platform its created on. That also makes it different
> > from all other data, AVPacket.data and extradata are all
> > platform independant. And there is no API to
> > convert it into a platform ABI independant form. 
> 
> Why should it be transferable over network? That makes no sense. We
> don't have the ability to transfer AVFrame, AVPacket, or AVCodecContext
> over network either.

If you have an application that streams video, and on a different computer
an application which plays that video you need to transfer the compressed
data (that is AVPackets and their side data) over the network.

You can use RT*P/hls and others but they lack the concept of our side data
so many cases will not work over such system, especially not if there is
no platform independant representation of the data which you have to
transmit between platforms.
At some point you have to 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[wm4]

On Mon, 18 Dec 2017 21:38:24 +0100
Michael Niedermayer  wrote:

> On Mon, Dec 18, 2017 at 04:56:08PM -0300, James Almer wrote:
> > On 12/18/2017 4:52 PM, wm4 wrote:  
> > > On Fri, 15 Dec 2017 14:24:17 -0800
> > > Jacob Trimble  wrote:
> > >   
> > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> > >> From: Jacob Trimble 
> > >> Date: Tue, 5 Dec 2017 14:52:22 -0800
> > >> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
> > >>
> > >> This new side-data will contain info on how a packet is encrypted.
> > >> This allows the app to handle packet decryption.  To allow for a
> > >> variable number of subsamples, the buffer for the side-data will be
> > >> allocated to hold both the structure and the array of subsamples.  So
> > >> the |subsamples| member will point to right after the struct.
> > >>
> > >> Signed-off-by: Jacob Trimble 
> > >> ---
> > >>  libavcodec/avcodec.h | 70 
> > >> 
> > >>  1 file changed, 70 insertions(+)
> > >>
> > >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > >> index 5db6a81320..ccc89345e8 100644
> > >> --- a/libavcodec/avcodec.h
> > >> +++ b/libavcodec/avcodec.h
> > >> @@ -1112,6 +1112,63 @@ typedef struct AVCPBProperties {
> > >>  uint64_t vbv_delay;
> > >>  } AVCPBProperties;
> > >>  
> > >> +typedef struct AVPacketSubsampleEncryptionInfo {
> > >> +/** The number of bytes that are clear. */
> > >> +unsigned int bytes_of_clear_data;
> > >> +
> > >> +/**
> > >> + * The number of bytes that are protected.  If using pattern 
> > >> encryption,
> > >> + * the pattern applies to only the protected bytes; if not using 
> > >> pattern
> > >> + * encryption, all these bytes are encrypted.
> > >> + */
> > >> +unsigned int bytes_of_protected_data;
> > >> +} AVPacketSubsampleEncryptionInfo;
> > >> +
> > >> +/**
> > >> + * This describes encryption info for a packet.  This contains 
> > >> frame-specific
> > >> + * info for how to decrypt the packet before passing it to the decoder. 
> > >>  If this
> > >> + * side-data is present, then the packet is encrypted.
> > >> + */
> > >> +typedef struct AVPacketEncryptionInfo {
> > >> +/** The fourcc encryption scheme. */
> > >> +uint32_t scheme;
> > >> +
> > >> +/** The ID of the key used to encrypt the packet. */
> > >> +uint8_t key_id[16];
> > >> +
> > >> +/** The initialization vector. */
> > >> +uint8_t iv[16];
> > >> +
> > >> +/**
> > >> + * Only used for pattern encryption.  This is the number of 16-byte 
> > >> blocks
> > >> + * that are encrypted.
> > >> + */
> > >> +unsigned int crypt_byte_block;
> > >> +
> > >> +/**
> > >> + * Only used for pattern encryption.  This is the number of 16-byte 
> > >> blocks
> > >> + * that are clear.
> > >> + */
> > >> +unsigned int skip_byte_block;
> > >> +
> > >> +/**
> > >> + * The number of sub-samples in this packet.  If 0, then the whole 
> > >> sample
> > >> + * is encrypted.
> > >> + */
> > >> +unsigned int subsample_count;
> > >> +
> > >> +/** The subsample encryption info. */
> > >> +AVPacketSubsampleEncryptionInfo *subsamples;  
> > > 
> > > I don't think this is sane. So far, side data could simply be copied
> > > with memcpy, and having pointers to non-static data in side data would
> > > break this completely.  
> > 
> > Even more reasons to ditch the current side data API and come up with a
> > better designed one that can also be reused for packet, frame and
> > container needs.  
> 
> yes, 
> also if its redesigned, it should become possible to pass it over the
> network. Currently all side data depends on the platform ABI, so its
> only valid on the platform its created on. That also makes it different
> from all other data, AVPacket.data and extradata are all
> platform independant. And there is no API to
> convert it into a platform ABI independant form. 

Why should it be transferable over network? That makes no sense. We
don't have the ability to transfer AVFrame, AVPacket, or AVCodecContext
over network either.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[wm4]

On Mon, 18 Dec 2017 16:56:08 -0300
James Almer  wrote:

> On 12/18/2017 4:52 PM, wm4 wrote:
> > On Fri, 15 Dec 2017 14:24:17 -0800
> > Jacob Trimble  wrote:
> >   
> >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> >> From: Jacob Trimble 
> >> Date: Tue, 5 Dec 2017 14:52:22 -0800
> >> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
> >>
> >> This new side-data will contain info on how a packet is encrypted.
> >> This allows the app to handle packet decryption.  To allow for a
> >> variable number of subsamples, the buffer for the side-data will be
> >> allocated to hold both the structure and the array of subsamples.  So
> >> the |subsamples| member will point to right after the struct.
> >>
> >> Signed-off-by: Jacob Trimble 
> >> ---
> >>  libavcodec/avcodec.h | 70 
> >> 
> >>  1 file changed, 70 insertions(+)
> >>
> >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> >> index 5db6a81320..ccc89345e8 100644
> >> --- a/libavcodec/avcodec.h
> >> +++ b/libavcodec/avcodec.h
> >> @@ -1112,6 +1112,63 @@ typedef struct AVCPBProperties {
> >>  uint64_t vbv_delay;
> >>  } AVCPBProperties;
> >>  
> >> +typedef struct AVPacketSubsampleEncryptionInfo {
> >> +/** The number of bytes that are clear. */
> >> +unsigned int bytes_of_clear_data;
> >> +
> >> +/**
> >> + * The number of bytes that are protected.  If using pattern 
> >> encryption,
> >> + * the pattern applies to only the protected bytes; if not using 
> >> pattern
> >> + * encryption, all these bytes are encrypted.
> >> + */
> >> +unsigned int bytes_of_protected_data;
> >> +} AVPacketSubsampleEncryptionInfo;
> >> +
> >> +/**
> >> + * This describes encryption info for a packet.  This contains 
> >> frame-specific
> >> + * info for how to decrypt the packet before passing it to the decoder.  
> >> If this
> >> + * side-data is present, then the packet is encrypted.
> >> + */
> >> +typedef struct AVPacketEncryptionInfo {
> >> +/** The fourcc encryption scheme. */
> >> +uint32_t scheme;
> >> +
> >> +/** The ID of the key used to encrypt the packet. */
> >> +uint8_t key_id[16];
> >> +
> >> +/** The initialization vector. */
> >> +uint8_t iv[16];
> >> +
> >> +/**
> >> + * Only used for pattern encryption.  This is the number of 16-byte 
> >> blocks
> >> + * that are encrypted.
> >> + */
> >> +unsigned int crypt_byte_block;
> >> +
> >> +/**
> >> + * Only used for pattern encryption.  This is the number of 16-byte 
> >> blocks
> >> + * that are clear.
> >> + */
> >> +unsigned int skip_byte_block;
> >> +
> >> +/**
> >> + * The number of sub-samples in this packet.  If 0, then the whole 
> >> sample
> >> + * is encrypted.
> >> + */
> >> +unsigned int subsample_count;
> >> +
> >> +/** The subsample encryption info. */
> >> +AVPacketSubsampleEncryptionInfo *subsamples;  
> > 
> > I don't think this is sane. So far, side data could simply be copied
> > with memcpy, and having pointers to non-static data in side data would
> > break this completely.  
> 
> Even more reasons to ditch the current side data API and come up with a
> better designed one that can also be reused for packet, frame and
> container needs.

I fully agree. But redesigning the entire side data API for such a
feature is probably a bit too much to ask. On the other hand I would
have no problem holding back such an obscure feature for a while...

On that note, I wonder if we should add an AVPacket reserved field for
this, just so we don't have to wait for the next ABI bump.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Michael Niedermayer]

On Mon, Dec 18, 2017 at 04:56:08PM -0300, James Almer wrote:
> On 12/18/2017 4:52 PM, wm4 wrote:
> > On Fri, 15 Dec 2017 14:24:17 -0800
> > Jacob Trimble  wrote:
> > 
> >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> >> From: Jacob Trimble 
> >> Date: Tue, 5 Dec 2017 14:52:22 -0800
> >> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
> >>
> >> This new side-data will contain info on how a packet is encrypted.
> >> This allows the app to handle packet decryption.  To allow for a
> >> variable number of subsamples, the buffer for the side-data will be
> >> allocated to hold both the structure and the array of subsamples.  So
> >> the |subsamples| member will point to right after the struct.
> >>
> >> Signed-off-by: Jacob Trimble 
> >> ---
> >>  libavcodec/avcodec.h | 70 
> >> 
> >>  1 file changed, 70 insertions(+)
> >>
> >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> >> index 5db6a81320..ccc89345e8 100644
> >> --- a/libavcodec/avcodec.h
> >> +++ b/libavcodec/avcodec.h
> >> @@ -1112,6 +1112,63 @@ typedef struct AVCPBProperties {
> >>  uint64_t vbv_delay;
> >>  } AVCPBProperties;
> >>  
> >> +typedef struct AVPacketSubsampleEncryptionInfo {
> >> +/** The number of bytes that are clear. */
> >> +unsigned int bytes_of_clear_data;
> >> +
> >> +/**
> >> + * The number of bytes that are protected.  If using pattern 
> >> encryption,
> >> + * the pattern applies to only the protected bytes; if not using 
> >> pattern
> >> + * encryption, all these bytes are encrypted.
> >> + */
> >> +unsigned int bytes_of_protected_data;
> >> +} AVPacketSubsampleEncryptionInfo;
> >> +
> >> +/**
> >> + * This describes encryption info for a packet.  This contains 
> >> frame-specific
> >> + * info for how to decrypt the packet before passing it to the decoder.  
> >> If this
> >> + * side-data is present, then the packet is encrypted.
> >> + */
> >> +typedef struct AVPacketEncryptionInfo {
> >> +/** The fourcc encryption scheme. */
> >> +uint32_t scheme;
> >> +
> >> +/** The ID of the key used to encrypt the packet. */
> >> +uint8_t key_id[16];
> >> +
> >> +/** The initialization vector. */
> >> +uint8_t iv[16];
> >> +
> >> +/**
> >> + * Only used for pattern encryption.  This is the number of 16-byte 
> >> blocks
> >> + * that are encrypted.
> >> + */
> >> +unsigned int crypt_byte_block;
> >> +
> >> +/**
> >> + * Only used for pattern encryption.  This is the number of 16-byte 
> >> blocks
> >> + * that are clear.
> >> + */
> >> +unsigned int skip_byte_block;
> >> +
> >> +/**
> >> + * The number of sub-samples in this packet.  If 0, then the whole 
> >> sample
> >> + * is encrypted.
> >> + */
> >> +unsigned int subsample_count;
> >> +
> >> +/** The subsample encryption info. */
> >> +AVPacketSubsampleEncryptionInfo *subsamples;
> > 
> > I don't think this is sane. So far, side data could simply be copied
> > with memcpy, and having pointers to non-static data in side data would
> > break this completely.
> 
> Even more reasons to ditch the current side data API and come up with a
> better designed one that can also be reused for packet, frame and
> container needs.

yes, 
also if its redesigned, it should become possible to pass it over the
network. Currently all side data depends on the platform ABI, so its
only valid on the platform its created on. That also makes it different
from all other data, AVPacket.data and extradata are all
platform independant. And there is no API to
convert it into a platform ABI independant form. 



[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle


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

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[James Almer]

On 12/18/2017 4:52 PM, wm4 wrote:
> On Fri, 15 Dec 2017 14:24:17 -0800
> Jacob Trimble  wrote:
> 
>> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
>> From: Jacob Trimble 
>> Date: Tue, 5 Dec 2017 14:52:22 -0800
>> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
>>
>> This new side-data will contain info on how a packet is encrypted.
>> This allows the app to handle packet decryption.  To allow for a
>> variable number of subsamples, the buffer for the side-data will be
>> allocated to hold both the structure and the array of subsamples.  So
>> the |subsamples| member will point to right after the struct.
>>
>> Signed-off-by: Jacob Trimble 
>> ---
>>  libavcodec/avcodec.h | 70 
>> 
>>  1 file changed, 70 insertions(+)
>>
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index 5db6a81320..ccc89345e8 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -1112,6 +1112,63 @@ typedef struct AVCPBProperties {
>>  uint64_t vbv_delay;
>>  } AVCPBProperties;
>>  
>> +typedef struct AVPacketSubsampleEncryptionInfo {
>> +/** The number of bytes that are clear. */
>> +unsigned int bytes_of_clear_data;
>> +
>> +/**
>> + * The number of bytes that are protected.  If using pattern encryption,
>> + * the pattern applies to only the protected bytes; if not using pattern
>> + * encryption, all these bytes are encrypted.
>> + */
>> +unsigned int bytes_of_protected_data;
>> +} AVPacketSubsampleEncryptionInfo;
>> +
>> +/**
>> + * This describes encryption info for a packet.  This contains 
>> frame-specific
>> + * info for how to decrypt the packet before passing it to the decoder.  If 
>> this
>> + * side-data is present, then the packet is encrypted.
>> + */
>> +typedef struct AVPacketEncryptionInfo {
>> +/** The fourcc encryption scheme. */
>> +uint32_t scheme;
>> +
>> +/** The ID of the key used to encrypt the packet. */
>> +uint8_t key_id[16];
>> +
>> +/** The initialization vector. */
>> +uint8_t iv[16];
>> +
>> +/**
>> + * Only used for pattern encryption.  This is the number of 16-byte 
>> blocks
>> + * that are encrypted.
>> + */
>> +unsigned int crypt_byte_block;
>> +
>> +/**
>> + * Only used for pattern encryption.  This is the number of 16-byte 
>> blocks
>> + * that are clear.
>> + */
>> +unsigned int skip_byte_block;
>> +
>> +/**
>> + * The number of sub-samples in this packet.  If 0, then the whole 
>> sample
>> + * is encrypted.
>> + */
>> +unsigned int subsample_count;
>> +
>> +/** The subsample encryption info. */
>> +AVPacketSubsampleEncryptionInfo *subsamples;
> 
> I don't think this is sane. So far, side data could simply be copied
> with memcpy, and having pointers to non-static data in side data would
> break this completely.

Even more reasons to ditch the current side data API and come up with a
better designed one that can also be reused for packet, frame and
container needs.

> 
>> +} AVPacketEncryptionInfo;
>> +/**
>> + * The size of the side-data for the AV_PKT_DATA_PACKET_ENCRYPTION_INFO 
>> type.
>> + * The side-data will contain the AVPacketEncryptionInfo struct followed by
>> + * the subsample array.  The subsamples member should point to after the 
>> struct
>> + * so the app can easily access it.
>> + */
>> +#define FF_PACKET_ENCRYPTION_INFO_SIZE(subsample_count) \
>> +(sizeof(AVPacketEncryptionInfo) + 
>> sizeof(AVPacketSubsampleEncryptionInfo) * subsample_count)
>> +
>>  /**
>>   * The decoder will keep a reference to the frame and may reuse it later.
>>   */
>> @@ -1327,6 +1384,19 @@ enum AVPacketSideDataType {
>>   */
>>  AV_PKT_DATA_A53_CC,
>>  
>> +/**
>> + * This side data is encryption "initialization data".
>> + * For MP4 this is the entire 'pssh' box.
>> + * For WebM this is the key ID.
>> + */
>> +AV_PKT_DATA_ENCRYPTION_INIT_DATA,
>> +
>> +/**
>> + * This side data is an AVPacketEncryptionInfo structure and contains 
>> info
>> + * about how the packet is encrypted.
>> + */
>> +AV_PKT_DATA_PACKET_ENCRYPTION_INFO,
>> +
>>  /**
>>   * The number of side data types.
>>   * This is not part of the public API/ABI in the sense that it may
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

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

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[wm4]

On Fri, 15 Dec 2017 14:24:17 -0800
Jacob Trimble  wrote:

> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> From: Jacob Trimble 
> Date: Tue, 5 Dec 2017 14:52:22 -0800
> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
> 
> This new side-data will contain info on how a packet is encrypted.
> This allows the app to handle packet decryption.  To allow for a
> variable number of subsamples, the buffer for the side-data will be
> allocated to hold both the structure and the array of subsamples.  So
> the |subsamples| member will point to right after the struct.
> 
> Signed-off-by: Jacob Trimble 
> ---
>  libavcodec/avcodec.h | 70 
> 
>  1 file changed, 70 insertions(+)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 5db6a81320..ccc89345e8 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -1112,6 +1112,63 @@ typedef struct AVCPBProperties {
>  uint64_t vbv_delay;
>  } AVCPBProperties;
>  
> +typedef struct AVPacketSubsampleEncryptionInfo {
> +/** The number of bytes that are clear. */
> +unsigned int bytes_of_clear_data;
> +
> +/**
> + * The number of bytes that are protected.  If using pattern encryption,
> + * the pattern applies to only the protected bytes; if not using pattern
> + * encryption, all these bytes are encrypted.
> + */
> +unsigned int bytes_of_protected_data;
> +} AVPacketSubsampleEncryptionInfo;
> +
> +/**
> + * This describes encryption info for a packet.  This contains frame-specific
> + * info for how to decrypt the packet before passing it to the decoder.  If 
> this
> + * side-data is present, then the packet is encrypted.
> + */
> +typedef struct AVPacketEncryptionInfo {
> +/** The fourcc encryption scheme. */
> +uint32_t scheme;
> +
> +/** The ID of the key used to encrypt the packet. */
> +uint8_t key_id[16];
> +
> +/** The initialization vector. */
> +uint8_t iv[16];
> +
> +/**
> + * Only used for pattern encryption.  This is the number of 16-byte 
> blocks
> + * that are encrypted.
> + */
> +unsigned int crypt_byte_block;
> +
> +/**
> + * Only used for pattern encryption.  This is the number of 16-byte 
> blocks
> + * that are clear.
> + */
> +unsigned int skip_byte_block;
> +
> +/**
> + * The number of sub-samples in this packet.  If 0, then the whole sample
> + * is encrypted.
> + */
> +unsigned int subsample_count;
> +
> +/** The subsample encryption info. */
> +AVPacketSubsampleEncryptionInfo *subsamples;

I don't think this is sane. So far, side data could simply be copied
with memcpy, and having pointers to non-static data in side data would
break this completely.

> +} AVPacketEncryptionInfo;
> +/**
> + * The size of the side-data for the AV_PKT_DATA_PACKET_ENCRYPTION_INFO type.
> + * The side-data will contain the AVPacketEncryptionInfo struct followed by
> + * the subsample array.  The subsamples member should point to after the 
> struct
> + * so the app can easily access it.
> + */
> +#define FF_PACKET_ENCRYPTION_INFO_SIZE(subsample_count) \
> +(sizeof(AVPacketEncryptionInfo) + 
> sizeof(AVPacketSubsampleEncryptionInfo) * subsample_count)
> +
>  /**
>   * The decoder will keep a reference to the frame and may reuse it later.
>   */
> @@ -1327,6 +1384,19 @@ enum AVPacketSideDataType {
>   */
>  AV_PKT_DATA_A53_CC,
>  
> +/**
> + * This side data is encryption "initialization data".
> + * For MP4 this is the entire 'pssh' box.
> + * For WebM this is the key ID.
> + */
> +AV_PKT_DATA_ENCRYPTION_INIT_DATA,
> +
> +/**
> + * This side data is an AVPacketEncryptionInfo structure and contains 
> info
> + * about how the packet is encrypted.
> + */
> +AV_PKT_DATA_PACKET_ENCRYPTION_INFO,
> +
>  /**
>   * The number of side data types.
>   * This is not part of the public API/ABI in the sense that it may

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

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Jacob Trimble]

>> @@ -1327,6 +1384,19 @@ enum AVPacketSideDataType {
>>   */
>>  AV_PKT_DATA_A53_CC,
>>
>> +/**
>> + * This side data is encryption "initialization data".
>> + * For MP4 this is the entire 'pssh' box.
>> + * For WebM this is the key ID.
>> + */
>> +AV_PKT_DATA_ENCRYPTION_INIT_DATA,
>
> So its basically like extradata is for a codec ?
> If so it should be defined similarly as opaque encryption scheme specific 
> data.
> It should not be container specific.
> Data taken from one container should be storable in another if both support
> the features used

Yes, that's the idea.  Unfortunately the data is specific to a
container.  This isn't taken from the common encryption spec, but from
the EME spec.  The "EME initialization data registry" specifies
several fixed formats for initialization data that are all specific to
a container.  

It would be possible to parse the PSSH boxes and just push the generic
data to the app.  This would allow new containers to hold the same
data; then the app could wrap it back in a generic PSSH box for EME.
But that would remove the key ID information that exists in v1 PSSH
boxes.  Some apps parse the PSSH boxes and filter the initialization
based on whether they already have those key IDs to avoid creating too
many sessions.  So I would like to expose the whole PSSH box in the
side data so apps can do that.

I can change this to something that is specific to MP4 instead (e.g.
AV_PKT_DATA_MP4_PSSH).  The app could just pull the key ID from the
first sample in the case of WebM.

>
> thanks
>
> [...]
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Michael Niedermayer]

On Fri, Dec 15, 2017 at 02:24:17PM -0800, Jacob Trimble wrote:
> >> +
> >> +/** The ID of the key used to encrypt the packet. */
> >> +uint8_t key_id[16];
> >> +
> >> +/** The initialization vector. */
> >> +uint8_t iv[16];
> >
> > what happens if the key or iv is longer ?
> 
> Both are specified by common encryption to be exactly that size.  To
> be able to be
> longer would require an updated spec.  Plus it avoids having to store a 
> dynamic
> string somewhere in the side data.

It is difficult to change side data in the future as it is part of the ABI
Including a value representing the block size even if you only support 16 would
make it less painfull if it needs to be extended in the future.


[...]

> >
> >
> >> +
> >> +/**
> >> + * The number of sub-samples in this packet.  If 0, then the whole 
> >> sample
> >> + * is encrypted.
> >> + */
> >> +unsigned int subsample_count;
> >> +
> >> +struct {
> >> +  /** The number of bytes that are clear. */
> >> +  unsigned int bytes_of_clear_data;
> >> +
> >> +  /**
> >> +   * The number of bytes that are protected.  If using pattern 
> >> encryption,
> >> +   * the pattern applies to only the protected bytes; if not using 
> >> pattern
> >> +   * encryption, all these bytes are encrypted.
> >> +   */
> >> +  unsigned int bytes_of_protected_data;
> >> +} *subsamples;
> >
> > Are these patterns random in practice ?
> > if not they possibly would be better not repeated per packet
> 
> They are unique per packet.
> 
> For example, in H.264, certain NAL unit types should be kept in the clear, 
> while
> others should be encrypted.  Plus even for encrypted NAL units, the header
> should be kept in the clear even when the payload is encrypted.

ok


[...]

> @@ -1327,6 +1384,19 @@ enum AVPacketSideDataType {
>   */
>  AV_PKT_DATA_A53_CC,
>  
> +/**
> + * This side data is encryption "initialization data".
> + * For MP4 this is the entire 'pssh' box.
> + * For WebM this is the key ID.
> + */
> +AV_PKT_DATA_ENCRYPTION_INIT_DATA,

So its basically like extradata is for a codec ?
If so it should be defined similarly as opaque encryption scheme specific data.
It should not be container specific.
Data taken from one container should be storable in another if both support
the features used

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire


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

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Jacob Trimble]

>> +
>> +/** The ID of the key used to encrypt the packet. */
>> +uint8_t key_id[16];
>> +
>> +/** The initialization vector. */
>> +uint8_t iv[16];
>
> what happens if the key or iv is longer ?

Both are specified by common encryption to be exactly that size.  To
be able to be
longer would require an updated spec.  Plus it avoids having to store a dynamic
string somewhere in the side data.

>
>> +
>> +/**
>> + * Only used for pattern encryption.  This is the number of 16-byte 
>> blocks
>> + * that are encrypted.
>> + */
>> +unsigned int crypt_byte_block;
>> +
>> +/**
>> + * Only used for pattern encryption.  This is the number of 16-byte 
>> blocks
>> + * that are clear.
>> + */
>> +unsigned int skip_byte_block;
>
> why is "16-byte blocks" hardcoded ?

Because that is how pattern encryption works in common encryption.  It
is specified
by number of AES blocks, not individual bytes.  I could change to
bytes, but it would
likely require the app to divide by 16 later and it runs the risk of
the value not being a
multiple of 16 like the spec requires.

>
>
>> +
>> +/**
>> + * The number of sub-samples in this packet.  If 0, then the whole 
>> sample
>> + * is encrypted.
>> + */
>> +unsigned int subsample_count;
>> +
>> +struct {
>> +  /** The number of bytes that are clear. */
>> +  unsigned int bytes_of_clear_data;
>> +
>> +  /**
>> +   * The number of bytes that are protected.  If using pattern 
>> encryption,
>> +   * the pattern applies to only the protected bytes; if not using 
>> pattern
>> +   * encryption, all these bytes are encrypted.
>> +   */
>> +  unsigned int bytes_of_protected_data;
>> +} *subsamples;
>
> Are these patterns random in practice ?
> if not they possibly would be better not repeated per packet

They are unique per packet.

For example, in H.264, certain NAL unit types should be kept in the clear, while
others should be encrypted.  Plus even for encrypted NAL units, the header
should be kept in the clear even when the payload is encrypted.

>
>
>> +} AVPacketEncryptionInfo;
>
>> +#define FF_PACKET_ENCRYPTION_INFO_SIZE(a) (sizeof(AVPacketEncryptionInfo) + 
>> sizeof(unsigned int) * a * 2)
>
> This assumes things about the padding and alignment of fields that are not
> guranteed by C i think
>

I was trying to avoid having another top-level struct.  Too bad you
can get the size
of members without an instance in c.
From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
Date: Tue, 5 Dec 2017 14:52:22 -0800
Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.

This new side-data will contain info on how a packet is encrypted.
This allows the app to handle packet decryption.  To allow for a
variable number of subsamples, the buffer for the side-data will be
allocated to hold both the structure and the array of subsamples.  So
the |subsamples| member will point to right after the struct.

Signed-off-by: Jacob Trimble 
---
 libavcodec/avcodec.h | 70 
 1 file changed, 70 insertions(+)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 5db6a81320..ccc89345e8 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1112,6 +1112,63 @@ typedef struct AVCPBProperties {
 uint64_t vbv_delay;
 } AVCPBProperties;
 
+typedef struct AVPacketSubsampleEncryptionInfo {
+/** The number of bytes that are clear. */
+unsigned int bytes_of_clear_data;
+
+/**
+ * The number of bytes that are protected.  If using pattern encryption,
+ * the pattern applies to only the protected bytes; if not using pattern
+ * encryption, all these bytes are encrypted.
+ */
+unsigned int bytes_of_protected_data;
+} AVPacketSubsampleEncryptionInfo;
+
+/**
+ * This describes encryption info for a packet.  This contains frame-specific
+ * info for how to decrypt the packet before passing it to the decoder.  If this
+ * side-data is present, then the packet is encrypted.
+ */
+typedef struct AVPacketEncryptionInfo {
+/** The fourcc encryption scheme. */
+uint32_t scheme;
+
+/** The ID of the key used to encrypt the packet. */
+uint8_t key_id[16];
+
+/** The initialization vector. */
+uint8_t iv[16];
+
+/**
+ * Only used for pattern encryption.  This is the number of 16-byte blocks
+ * that are encrypted.
+ */
+unsigned int crypt_byte_block;
+
+/**
+ * Only used for pattern encryption.  This is the number of 16-byte blocks
+ * that are clear.
+ */
+unsigned int skip_byte_block;
+
+/**
+ * The number of sub-samples in this packet.  If 0, then the whole sample
+ * is encrypted.
+ */
+unsigned int subsample_count;
+
+/** The subsample encryption info. */
+AVPacketSubsampleEncryptionInfo *subsamples;
+} AVPacketEncryptionInfo;
+/**
+ * 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Michael Niedermayer]

On Fri, Dec 08, 2017 at 10:06:13AM -0800, Jacob Trimble wrote:
> On Tue, Dec 5, 2017 at 5:22 PM, Derek Buitenhuis
>  wrote:
> > On 12/6/2017 12:36 AM, Jacob Trimble wrote:
> >> Would a 0-length array work?  Otherwise I would need to have it be a
> >> 1-length array and have to account for that when calculating the size
> >> to allocate; it would also require a comment to ignore the size of the
> >> array.
> >
> > Aren't 0-length arrays a GNU extensions? If so, I would gather that it
> > probably is not OK in a public header, either.
> 
> Yeah.
> 
> > I'm not entirely sure what way we prefer nowadays, but you can see
> > we've had side data with variable length members before with e.g.
> > AV_PKT_DATA_QUALITY_STATS, but that doesn't have a struct associated
> > with it. I'm hoping someone more up to date with the side data stuff
> > can chime in with a suggestion on what our current best practices
> > are for it.
> 
> I would prefer to avoid requiring the app to "parse" the side data, using a
> plain struct would be better.  I've updated the patch to include my other plan
> of allocating a larger bock and having the struct followed by the subsample
> array.
> 
> I have also renamed the file, I think the mailing list rejected my attachment
> before since gmail sent the MIME type as text/x-patch.  Hopefully with the
> .txt extension gmail will send the correct MIME type.

> From 740c0ccf3ed90b3d417ea25ec26cfefb93974461 Mon Sep 17 00:00:00 2001
> From: Jacob Trimble 
> Date: Tue, 5 Dec 2017 14:52:22 -0800
> Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.
> 
> This new side-data will contain info on how a packet is encrypted.
> This allows the app to handle packet decryption.  To allow for a
> variable number of subsamples, the buffer for the side-data will be
> allocated to hold both the structure and the array of subsamples.  So
> the |subsamples| member will point to right after the struct.
> 
> Signed-off-by: Jacob Trimble 
> ---
>  libavcodec/avcodec.h | 60 
> 
>  1 file changed, 60 insertions(+)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 5db6a81320..6f5c387556 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -1112,6 +1112,53 @@ typedef struct AVCPBProperties {
>  uint64_t vbv_delay;
>  } AVCPBProperties;
>  
> +/**
> + * This describes encryption info for a packet.  This contains frame-specific
> + * info for how to decrypt the packet before passing it to the decoder.  If 
> this
> + * side-data is present, then the packet is encrypted.
> + */

> +typedef struct AVPacketEncryptionInfo {
> +/** The fourcc encryption scheme. */
> +int scheme;

uint32_t or unsigned


> +
> +/** The ID of the key used to encrypt the packet. */
> +uint8_t key_id[16];
> +
> +/** The initialization vector. */
> +uint8_t iv[16];

what happens if the key or iv is longer ?


> +
> +/**
> + * Only used for pattern encryption.  This is the number of 16-byte 
> blocks
> + * that are encrypted.
> + */
> +unsigned int crypt_byte_block;
> +
> +/**
> + * Only used for pattern encryption.  This is the number of 16-byte 
> blocks
> + * that are clear.
> + */
> +unsigned int skip_byte_block;

why is "16-byte blocks" hardcoded ?


> +
> +/**
> + * The number of sub-samples in this packet.  If 0, then the whole sample
> + * is encrypted.
> + */
> +unsigned int subsample_count;
> +
> +struct {
> +  /** The number of bytes that are clear. */
> +  unsigned int bytes_of_clear_data;
> +
> +  /**
> +   * The number of bytes that are protected.  If using pattern 
> encryption,
> +   * the pattern applies to only the protected bytes; if not using 
> pattern
> +   * encryption, all these bytes are encrypted.
> +   */
> +  unsigned int bytes_of_protected_data;
> +} *subsamples;

Are these patterns random in practice ?
if not they possibly would be better not repeated per packet


> +} AVPacketEncryptionInfo;

> +#define FF_PACKET_ENCRYPTION_INFO_SIZE(a) (sizeof(AVPacketEncryptionInfo) + 
> sizeof(unsigned int) * a * 2)

This assumes things about the padding and alignment of fields that are not
guranteed by C i think


> +
>  /**
>   * The decoder will keep a reference to the frame and may reuse it later.
>   */
> @@ -1327,6 +1374,19 @@ enum AVPacketSideDataType {
>   */
>  AV_PKT_DATA_A53_CC,
>  
> +/**
> + * This side data is encryption "initialization data".
> + * For MP4 this is the entire 'pssh' box.
> + * For WebM this is the key ID.
> + */
> +AV_PKT_DATA_ENCRYPTION_INIT_DATA,
> +
> +/**
> + * This side data is an AVPacketEncryptionInfo structure and contains 
> info
> + * about how the packet is encrypted.
> + */
> +AV_PKT_DATA_PACKET_ENCRYPTION_INFO,
> +
>  /**
>   * The 

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Jacob Trimble]

On Fri, Dec 8, 2017 at 10:06 AM, Jacob Trimble  wrote:
> On Tue, Dec 5, 2017 at 5:22 PM, Derek Buitenhuis
>  wrote:
>> On 12/6/2017 12:36 AM, Jacob Trimble wrote:
>>> Would a 0-length array work?  Otherwise I would need to have it be a
>>> 1-length array and have to account for that when calculating the size
>>> to allocate; it would also require a comment to ignore the size of the
>>> array.
>>
>> Aren't 0-length arrays a GNU extensions? If so, I would gather that it
>> probably is not OK in a public header, either.
>
> Yeah.
>
>> I'm not entirely sure what way we prefer nowadays, but you can see
>> we've had side data with variable length members before with e.g.
>> AV_PKT_DATA_QUALITY_STATS, but that doesn't have a struct associated
>> with it. I'm hoping someone more up to date with the side data stuff
>> can chime in with a suggestion on what our current best practices
>> are for it.
>
> I would prefer to avoid requiring the app to "parse" the side data, using a
> plain struct would be better.  I've updated the patch to include my other plan
> of allocating a larger bock and having the struct followed by the subsample
> array.
>
> I have also renamed the file, I think the mailing list rejected my attachment
> before since gmail sent the MIME type as text/x-patch.  Hopefully with the
> .txt extension gmail will send the correct MIME type.

Ping.  Is this something that can be pushed?  I have the implementation for
mov almost ready.  I think having this info exposed would be something that
would be really useful for the project.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Jacob Trimble]

On Tue, Dec 5, 2017 at 5:22 PM, Derek Buitenhuis
 wrote:
> On 12/6/2017 12:36 AM, Jacob Trimble wrote:
>> Would a 0-length array work?  Otherwise I would need to have it be a
>> 1-length array and have to account for that when calculating the size
>> to allocate; it would also require a comment to ignore the size of the
>> array.
>
> Aren't 0-length arrays a GNU extensions? If so, I would gather that it
> probably is not OK in a public header, either.

Yeah.

> I'm not entirely sure what way we prefer nowadays, but you can see
> we've had side data with variable length members before with e.g.
> AV_PKT_DATA_QUALITY_STATS, but that doesn't have a struct associated
> with it. I'm hoping someone more up to date with the side data stuff
> can chime in with a suggestion on what our current best practices
> are for it.

I would prefer to avoid requiring the app to "parse" the side data, using a
plain struct would be better.  I've updated the patch to include my other plan
of allocating a larger bock and having the struct followed by the subsample
array.

I have also renamed the file, I think the mailing list rejected my attachment
before since gmail sent the MIME type as text/x-patch.  Hopefully with the
.txt extension gmail will send the correct MIME type.
From 740c0ccf3ed90b3d417ea25ec26cfefb93974461 Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
Date: Tue, 5 Dec 2017 14:52:22 -0800
Subject: [PATCH] avcodec/avcodec.h: Add encryption info side data.

This new side-data will contain info on how a packet is encrypted.
This allows the app to handle packet decryption.  To allow for a
variable number of subsamples, the buffer for the side-data will be
allocated to hold both the structure and the array of subsamples.  So
the |subsamples| member will point to right after the struct.

Signed-off-by: Jacob Trimble 
---
 libavcodec/avcodec.h | 60 
 1 file changed, 60 insertions(+)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 5db6a81320..6f5c387556 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1112,6 +1112,53 @@ typedef struct AVCPBProperties {
 uint64_t vbv_delay;
 } AVCPBProperties;
 
+/**
+ * This describes encryption info for a packet.  This contains frame-specific
+ * info for how to decrypt the packet before passing it to the decoder.  If 
this
+ * side-data is present, then the packet is encrypted.
+ */
+typedef struct AVPacketEncryptionInfo {
+/** The fourcc encryption scheme. */
+int scheme;
+
+/** The ID of the key used to encrypt the packet. */
+uint8_t key_id[16];
+
+/** The initialization vector. */
+uint8_t iv[16];
+
+/**
+ * Only used for pattern encryption.  This is the number of 16-byte blocks
+ * that are encrypted.
+ */
+unsigned int crypt_byte_block;
+
+/**
+ * Only used for pattern encryption.  This is the number of 16-byte blocks
+ * that are clear.
+ */
+unsigned int skip_byte_block;
+
+/**
+ * The number of sub-samples in this packet.  If 0, then the whole sample
+ * is encrypted.
+ */
+unsigned int subsample_count;
+
+struct {
+  /** The number of bytes that are clear. */
+  unsigned int bytes_of_clear_data;
+
+  /**
+   * The number of bytes that are protected.  If using pattern encryption,
+   * the pattern applies to only the protected bytes; if not using pattern
+   * encryption, all these bytes are encrypted.
+   */
+  unsigned int bytes_of_protected_data;
+} *subsamples;
+} AVPacketEncryptionInfo;
+#define FF_PACKET_ENCRYPTION_INFO_SIZE(a) (sizeof(AVPacketEncryptionInfo) + 
sizeof(unsigned int) * a * 2)
+
 /**
  * The decoder will keep a reference to the frame and may reuse it later.
  */
@@ -1327,6 +1374,19 @@ enum AVPacketSideDataType {
  */
 AV_PKT_DATA_A53_CC,
 
+/**
+ * This side data is encryption "initialization data".
+ * For MP4 this is the entire 'pssh' box.
+ * For WebM this is the key ID.
+ */
+AV_PKT_DATA_ENCRYPTION_INIT_DATA,
+
+/**
+ * This side data is an AVPacketEncryptionInfo structure and contains info
+ * about how the packet is encrypted.
+ */
+AV_PKT_DATA_PACKET_ENCRYPTION_INFO,
+
 /**
  * The number of side data types.
  * This is not part of the public API/ABI in the sense that it may
-- 
2.15.1.424.g9478a66081-goog

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

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Derek Buitenhuis]

On 12/6/2017 12:36 AM, Jacob Trimble wrote:
> Would a 0-length array work?  Otherwise I would need to have it be a
> 1-length array and have to account for that when calculating the size
> to allocate; it would also require a comment to ignore the size of the
> array.

Aren't 0-length arrays a GNU extensions? If so, I would gather that it
probably is not OK in a public header, either.

> The reason I want it to be an array is because I want a variable
> number of elements, but I want the data contained in the struct.
> Since this will be extra data, AFAIK it will only be free()'d, so I
> can't use pointers or it will leak.
> 
> Another alternative would be to still malloc more than needed and have
> the memory past the struct be the array.  That seems like a hack, but
> would allow a simple free().  For example:

I'm not entirely sure what way we prefer nowadays, but you can see
we've had side data with variable length members before with e.g.
AV_PKT_DATA_QUALITY_STATS, but that doesn't have a struct associated
with it. I'm hoping someone more up to date with the side data stuff
can chime in with a suggestion on what our current best practices
are for it.

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

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Jacob Trimble]

On Tue, Dec 5, 2017 at 3:25 PM, Derek Buitenhuis
 wrote:
> On 12/5/2017 11:00 PM, Jacob Trimble wrote:
>> Also, can I use the flexible array member feature, it was introduced
>> in C99?  Would a 0-length array be better?
>
> No, I don't think this would be OK inside a public header, unfortunately.

Would a 0-length array work?  Otherwise I would need to have it be a
1-length array and have to account for that when calculating the size
to allocate; it would also require a comment to ignore the size of the
array.

The reason I want it to be an array is because I want a variable
number of elements, but I want the data contained in the struct.
Since this will be extra data, AFAIK it will only be free()'d, so I
can't use pointers or it will leak.

Another alternative would be to still malloc more than needed and have
the memory past the struct be the array.  That seems like a hack, but
would allow a simple free().  For example:

info = malloc(sizeof(AVPacketEncryptionInfo) +
sizeof(AVPacketSubsampleInfo) * num_subsamples);
info.subsamples = (uint8_t*)info + sizeof(AVPacketEncryptionInfo);
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec/avcodec.h: Add encryption info side data

[Derek Buitenhuis]

On 12/5/2017 11:00 PM, Jacob Trimble wrote:
> Also, can I use the flexible array member feature, it was introduced
> in C99?  Would a 0-length array be better?

No, I don't think this would be OK inside a public header, unfortunately.

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