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

>> +
>> +    /** 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.

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

[...]

>> @@ -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.  <https://www.w3.org/TR/eme-initdata-registry/>

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
>
> [...]
>

On Fri, 15 Dec 2017 14:24:17 -0800
Jacob Trimble <modmaker-at-google.com@ffmpeg.org> wrote:

> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> From: Jacob Trimble <modmaker@google.com>
> 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 <modmaker@google.com>
> ---
>  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

On 12/18/2017 4:52 PM, wm4 wrote:
> On Fri, 15 Dec 2017 14:24:17 -0800
> Jacob Trimble <modmaker-at-google.com@ffmpeg.org> wrote:
> 
>> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
>> From: Jacob Trimble <modmaker@google.com>
>> 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 <modmaker@google.com>
>> ---
>>  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
>

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 <modmaker-at-google.com@ffmpeg.org> wrote:
> > 
> >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> >> From: Jacob Trimble <modmaker@google.com>
> >> 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 <modmaker@google.com>
> >> ---
> >>  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. 



[...]

On Mon, 18 Dec 2017 16:56:08 -0300
James Almer <jamrial@gmail.com> wrote:

> On 12/18/2017 4:52 PM, wm4 wrote:
> > On Fri, 15 Dec 2017 14:24:17 -0800
> > Jacob Trimble <modmaker-at-google.com@ffmpeg.org> wrote:
> >   
> >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> >> From: Jacob Trimble <modmaker@google.com>
> >> 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 <modmaker@google.com>
> >> ---
> >>  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.

On Mon, 18 Dec 2017 21:38:24 +0100
Michael Niedermayer <michael@niedermayer.cc> 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 <modmaker-at-google.com@ffmpeg.org> wrote:
> > >   
> > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> > >> From: Jacob Trimble <modmaker@google.com>
> > >> 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 <modmaker@google.com>
> > >> ---
> > >>  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.

On Mon, Dec 18, 2017 at 10:02:52PM +0100, wm4 wrote:
> On Mon, 18 Dec 2017 21:38:24 +0100
> Michael Niedermayer <michael@niedermayer.cc> 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 <modmaker-at-google.com@ffmpeg.org> wrote:
> > > >   
> > > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> > > >> From: Jacob Trimble <modmaker@google.com>
> > > >> 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 <modmaker@google.com>
> > > >> ---
> > > >>  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 get the side data from one platforms format
to another.

ATM this is only possible by storing it in exactly the file format which
supports the exact set of side data used transfer that file to the other
platform and then read that in again. Thats a very heavyweight approuch
that does alot of steps which are not really needed.
For low latency lightweight streaming a more direct transfer of serialized
AVPackets and their side data over the network should work much better


[...]

On Mon, 18 Dec 2017 22:17:14 +0100
Michael Niedermayer <michael@niedermayer.cc> wrote:

> On Mon, Dec 18, 2017 at 10:02:52PM +0100, wm4 wrote:
> > On Mon, 18 Dec 2017 21:38:24 +0100
> > Michael Niedermayer <michael@niedermayer.cc> 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 <modmaker-at-google.com@ffmpeg.org> wrote:
> > > > >     
> > > > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> > > > >> From: Jacob Trimble <modmaker@google.com>
> > > > >> 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 <modmaker@google.com>
> > > > >> ---
> > > > >>  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.

Use a streaming protocol instead of something hacked together.

Also, design APIs for API users and the common case, not for ultra
obscure special cases that are just vague ideas floating around your
brain.

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

These protocols don't need side data.

> At some point you have to get the side data from one platforms format
> to another.

No, you don't.

> ATM this is only possible by storing it in exactly the file format which
> supports the exact set of side data used transfer that file to the other
> platform and then read that in again. Thats a very heavyweight approuch
> that does alot of steps which are not really needed.
> For low latency lightweight streaming a more direct transfer of serialized
> AVPackets and their side data over the network should work much better
> 
> 
> [...]

On Mon, Dec 18, 2017 at 10:28:14PM +0100, wm4 wrote:
> On Mon, 18 Dec 2017 22:17:14 +0100
> Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
> > On Mon, Dec 18, 2017 at 10:02:52PM +0100, wm4 wrote:
> > > On Mon, 18 Dec 2017 21:38:24 +0100
> > > Michael Niedermayer <michael@niedermayer.cc> 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 <modmaker-at-google.com@ffmpeg.org> wrote:
> > > > > >     
> > > > > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> > > > > >> From: Jacob Trimble <modmaker@google.com>
> > > > > >> 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 <modmaker@google.com>
> > > > > >> ---
> > > > > >>  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.
> 
> Use a streaming protocol instead of something hacked together.

This is about a streaming protocol. One that supports our side data
which no currently existing streaming protocol fully supports.

designing a new clean streaming protocol that would allow a
libavcodec encoder to get all data of its packets serialized, transmitted over
the network deserialized and decoded would be very interresting.

The existing streaming protocols are not exactly clean.
For example packaging data into a series of mpeg-ts files which are
then transferred over http (which was designed for web pages not multimedia)
also commonly called hls / http live streaming is not a clean format.
Its also not a generic format like for example matroska or nut is for files.
The others i know of also all suffer from other design issues.
We could design a better one. Of course this doesnt need to happen
on ffmpeg-devel if some people here object. Nor does this need to
be done by us or me but it would be beneficial if someone did design a
better streaming protocol.
The existing streaming protocols i know have all issues.

Once we have a generic and clean streaming format we need a way to put
side data in a platform independant way in it.
This is independant of how the streaming format is designed pretty much.
Its just that no current protocol supports transfering all the side data
we support.
Such a new protocol would then likely define the layout of each side data
type byte per byte like any other data in a protocol would be defined.


> 
> Also, design APIs for API users and the common case, not for ultra
> obscure special cases that are just vague ideas floating around your
> brain.
> 
> > 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.
> 
> These protocols don't need side data.

> 
> > At some point you have to get the side data from one platforms format
> > to another.
> 
> No, you don't.

You do, otherwise side data would not work at all.
Even storing side data in a mp4 or mkv file as currently done and then reading
this file back converts it between platforms.
The whole system of side data wouldnt work if there was no way
to convert it between 2 platform representations

If someone belives side data never needs to be converted between platforms
consider this:
A encoder produces side data, this is specific to the platform the encoder
runs on currently.

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
That is either file format or a network protocol or something else.
But if it doesnt get converted then its content is lost and the purpose that
side data had is not realized. Which may be minor or major depending on the
side data.


[...]

On Tue, 19 Dec 2017 00:00:15 +0100
Michael Niedermayer <michael@niedermayer.cc> wrote:

> On Mon, Dec 18, 2017 at 10:28:14PM +0100, wm4 wrote:
> > On Mon, 18 Dec 2017 22:17:14 +0100
> > Michael Niedermayer <michael@niedermayer.cc> wrote:
> >   
> > > On Mon, Dec 18, 2017 at 10:02:52PM +0100, wm4 wrote:  
> > > > On Mon, 18 Dec 2017 21:38:24 +0100
> > > > Michael Niedermayer <michael@niedermayer.cc> 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 <modmaker-at-google.com@ffmpeg.org> wrote:
> > > > > > >       
> > > > > > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> > > > > > >> From: Jacob Trimble <modmaker@google.com>
> > > > > > >> 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 <modmaker@google.com>
> > > > > > >> ---
> > > > > > >>  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.  
> > 
> > Use a streaming protocol instead of something hacked together.  
> 
> This is about a streaming protocol. One that supports our side data
> which no currently existing streaming protocol fully supports.

So? Side data is a ffmpeg internal concept.

If you want to create a streaming protocol that does whatever, feel
free to do it, but leave ffmpeg internals out of it.

> designing a new clean streaming protocol that would allow a
> libavcodec encoder to get all data of its packets serialized, transmitted over
> the network deserialized and decoded would be very interresting.
> The existing streaming protocols are not exactly clean.

Relying on ffmpeg internals is NOT clean.

> For example packaging data into a series of mpeg-ts files which are
> then transferred over http (which was designed for web pages not multimedia)
> also commonly called hls / http live streaming is not a clean format.
> Its also not a generic format like for example matroska or nut is for files.
> The others i know of also all suffer from other design issues.
> We could design a better one. Of course this doesnt need to happen
> on ffmpeg-devel if some people here object. Nor does this need to

My god, this is a big load of bullshit again. So because I refuse that
the ffmpeg API is made extra awkward++ we can't have a new spiffy
streaming format that will safe the world.

I guess it's completely worthless trying to argue with you, as shown
with plenty of other examples in the past. It's always the same
worthless bikeshedding, never conceding to the other side, obsessing
about minor issues, wanting to make the common case exceedingly complex
for obscure use cases, and so on.

So let me put this in another way: because of your demands there will
never be a better side data API that replace the current one (that is
full of shortcomings, as illustrated by this patch). And of course your
spiffy new streaming protocol (that suspiciously sounds like ffserver
reloaded) will never happen anyway. I mean, side data has existed for
almost a decade and was supposed to be ABI independent (aka network
safe, i.e. what you're demanding here), and it never happened.

> > > At some point you have to get the side data from one platforms format
> > > to another.  
> > 
> > No, you don't.  
> 
> You do, otherwise side data would not work at all.
> Even storing side data in a mp4 or mkv file as currently done and then reading
> this file back converts it between platforms.
> The whole system of side data wouldnt work if there was no way
> to convert it between 2 platform representations

Well, thanks for admitting that we do not need an "official" ABI
independent representation of side data, because its representations
can always be converted to whatever is needed.

> 
> If someone belives side data never needs to be converted between platforms
> consider this:
> A encoder produces side data, this is specific to the platform the encoder
> runs on currently.
> 
> 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
> That is either file format or a network protocol or something else.
> But if it doesnt get converted then its content is lost and the purpose that
> side data had is not realized. Which may be minor or major depending on the
> side data.

And this has nothing to do with anything either.

Hi,

On Mon, Dec 18, 2017 at 6:00 PM, Michael Niedermayer <michael@niedermayer.cc
> 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

2017-12-19 0:17 GMT+01:00 wm4 <nfxjfg@googlemail.com>:

[...]

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

Carl Eugen

On Tue, 19 Dec 2017 00:36:36 +0100
Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> 2017-12-19 0:17 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
> 
> [...]
> 
> 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.

On Tue, Dec 19, 2017 at 12:17:11AM +0100, wm4 wrote:
> On Tue, 19 Dec 2017 00:00:15 +0100
> Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
> > On Mon, Dec 18, 2017 at 10:28:14PM +0100, wm4 wrote:
> > > On Mon, 18 Dec 2017 22:17:14 +0100
> > > Michael Niedermayer <michael@niedermayer.cc> wrote:
> > >   
> > > > On Mon, Dec 18, 2017 at 10:02:52PM +0100, wm4 wrote:  
> > > > > On Mon, 18 Dec 2017 21:38:24 +0100
> > > > > Michael Niedermayer <michael@niedermayer.cc> 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 <modmaker-at-google.com@ffmpeg.org> wrote:
> > > > > > > >       
> > > > > > > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> > > > > > > >> From: Jacob Trimble <modmaker@google.com>
> > > > > > > >> 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 <modmaker@google.com>
> > > > > > > >> ---
> > > > > > > >>  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.  
> > > 
> > > Use a streaming protocol instead of something hacked together.  
> > 
> > This is about a streaming protocol. One that supports our side data
> > which no currently existing streaming protocol fully supports.
> 
> So? Side data is a ffmpeg internal concept.
> 
> If you want to create a streaming protocol that does whatever, feel
> free to do it, but leave ffmpeg internals out of it.
> 
> > designing a new clean streaming protocol that would allow a
> > libavcodec encoder to get all data of its packets serialized, transmitted over
> > the network deserialized and decoded would be very interresting.
> > The existing streaming protocols are not exactly clean.
> 
> Relying on ffmpeg internals is NOT clean.

I have not suggested to rely on FFmpeg internals.


> 
> > For example packaging data into a series of mpeg-ts files which are
> > then transferred over http (which was designed for web pages not multimedia)
> > also commonly called hls / http live streaming is not a clean format.
> > Its also not a generic format like for example matroska or nut is for files.
> > The others i know of also all suffer from other design issues.
> > We could design a better one. Of course this doesnt need to happen
> > on ffmpeg-devel if some people here object. Nor does this need to
> 
> My god, this is a big load of bullshit again. So because I refuse that
> the ffmpeg API is made extra awkward++ we can't have a new spiffy
> streaming format that will safe the world.
> 
> I guess it's completely worthless trying to argue with you, as shown
> with plenty of other examples in the past. It's always the same
> worthless bikeshedding, never conceding to the other side, obsessing
> about minor issues, wanting to make the common case exceedingly complex
> for obscure use cases, and so on.

I have no idea what you talk about


> 
> So let me put this in another way: because of your demands there will
> never be a better side data API that replace the current one (that is
> full of shortcomings, as illustrated by this patch). 

I have made no demand at all. In fact not even a concrete suggestion,
this is largly a discussion about a hypothetical streaming protocol.
For which serialization of some or all of the data we store in side data
would make sense.
The thread drifted to this as you asked
"Why should it be transferable over network? That makes no sense."

Because it of course does make sense, i explained why it makes sense, 
which of course became more off topic


> And of course your
> spiffy new streaming protocol (that suspiciously sounds like ffserver
> reloaded) will never happen anyway.

very possible. Most ideas never are turned into reality.
This was also just to explain why it makes sense to transfer some or all of
the information from side data over the network.


> I mean, side data has existed for
> almost a decade and was supposed to be ABI independent (aka network
> safe, i.e. what you're demanding here), and it never happened.

It was stored and loaded, but thats a whole subject of its own that would make
this even more off topic so lets stop it here

Thanks

also i appologize for anything that may have been unclear, incomplete or
misunderstood, i just tried to explain the case which you said "makes no sense"

[...]

On Tue, 19 Dec 2017 01:14:37 +0100
Michael Niedermayer <michael@niedermayer.cc> wrote:

> On Tue, Dec 19, 2017 at 12:17:11AM +0100, wm4 wrote:
> > On Tue, 19 Dec 2017 00:00:15 +0100
> > Michael Niedermayer <michael@niedermayer.cc> wrote:
> >   
> > > On Mon, Dec 18, 2017 at 10:28:14PM +0100, wm4 wrote:  
> > > > On Mon, 18 Dec 2017 22:17:14 +0100
> > > > Michael Niedermayer <michael@niedermayer.cc> wrote:
> > > >     
> > > > > On Mon, Dec 18, 2017 at 10:02:52PM +0100, wm4 wrote:    
> > > > > > On Mon, 18 Dec 2017 21:38:24 +0100
> > > > > > Michael Niedermayer <michael@niedermayer.cc> 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 <modmaker-at-google.com@ffmpeg.org> wrote:
> > > > > > > > >         
> > > > > > > > >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> > > > > > > > >> From: Jacob Trimble <modmaker@google.com>
> > > > > > > > >> 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 <modmaker@google.com>
> > > > > > > > >> ---
> > > > > > > > >>  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.    
> > > > 
> > > > Use a streaming protocol instead of something hacked together.    
> > > 
> > > This is about a streaming protocol. One that supports our side data
> > > which no currently existing streaming protocol fully supports.  
> > 
> > So? Side data is a ffmpeg internal concept.
> > 
> > If you want to create a streaming protocol that does whatever, feel
> > free to do it, but leave ffmpeg internals out of it.
> >   
> > > designing a new clean streaming protocol that would allow a
> > > libavcodec encoder to get all data of its packets serialized, transmitted over
> > > the network deserialized and decoded would be very interresting.
> > > The existing streaming protocols are not exactly clean.  
> > 
> > Relying on ffmpeg internals is NOT clean.  
> 
> I have not suggested to rely on FFmpeg internals.

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.

A reasonable protocol in practice would probably have some sort of
out-of-band signaling that fulfills a similar purpose as ffmpeg's side
data, but it would be a separate and different mechanism anyway.

> 
> >   
> > > For example packaging data into a series of mpeg-ts files which are
> > > then transferred over http (which was designed for web pages not multimedia)
> > > also commonly called hls / http live streaming is not a clean format.
> > > Its also not a generic format like for example matroska or nut is for files.
> > > The others i know of also all suffer from other design issues.
> > > We could design a better one. Of course this doesnt need to happen
> > > on ffmpeg-devel if some people here object. Nor does this need to  
> > 
> > My god, this is a big load of bullshit again. So because I refuse that
> > the ffmpeg API is made extra awkward++ we can't have a new spiffy
> > streaming format that will safe the world.
> > 
> > I guess it's completely worthless trying to argue with you, as shown
> > with plenty of other examples in the past. It's always the same
> > worthless bikeshedding, never conceding to the other side, obsessing
> > about minor issues, wanting to make the common case exceedingly complex
> > for obscure use cases, and so on.  
> 
> I have no idea what you talk about

My usual experience with you when you stubbornly insist on something.
Maybe I misinterpreted it this time and overreacted.

> 
> > 
> > So let me put this in another way: because of your demands there will
> > never be a better side data API that replace the current one (that is
> > full of shortcomings, as illustrated by this patch).   
> 
> I have made no demand at all. In fact not even a concrete suggestion,
> this is largly a discussion about a hypothetical streaming protocol.
> For which serialization of some or all of the data we store in side data
> would make sense.
> The thread drifted to this as you asked
> "Why should it be transferable over network? That makes no sense."
> 
> Because it of course does make sense, i explained why it makes sense, 
> which of course became more off topic

Anyway, of course the new side data API will have no mechanism for
serialization. But anyone is free to build one on top of it, of course,
just like current muxers/demuxers/codecs convert side data.

Side data is API, which means it tries to be some common denominator
for all the different features individual formats and protocols
provide. It's not suitable to implement some sort of super
protocol/format, because it would be too tightly coupled with ffmpeg
specifics, and would have to change as ffmpeg changes.

> 
> > And of course your
> > spiffy new streaming protocol (that suspiciously sounds like ffserver
> > reloaded) will never happen anyway.  
> 
> very possible. Most ideas never are turned into reality.
> This was also just to explain why it makes sense to transfer some or all of
> the information from side data over the network.
> 
> 
> > I mean, side data has existed for
> > almost a decade and was supposed to be ABI independent (aka network
> > safe, i.e. what you're demanding here), and it never happened.  
> 
> It was stored and loaded, but thats a whole subject of its own that would make
> this even more off topic so lets stop it here
> 
> Thanks
> 
> also i appologize for anything that may have been unclear, incomplete or
> misunderstood, i just tried to explain the case which you said "makes no sense"

It pretty much sounded to me like you wanted to make side data a wire
protocol (again), and expressed it as a strong opinion. Sorry if I
misunderstood and overreacted.

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 <michael@niedermayer.cc
> > 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

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

On Tue, 19 Dec 2017 14:20:38 -0800
Jacob Trimble <modmaker-at-google.com@ffmpeg.org> 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.

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.  <https://www.w3.org/TR/eme-initdata-registry/>
> 
> 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


[...]