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

[wm4]

On Mon, 18 Dec 2017 16:56:08 -0300
James Almer <jamrial at 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 at ffmpeg.org> wrote:
> >   
> >> From a1b2cbcb7da4da69685f8f1299b70b672ce448e3 Mon Sep 17 00:00:00 2001
> >> From: Jacob Trimble <modmaker at 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 at 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.