On Wed, 20 Dec 2017 15:10:43 -0800
Jacob Trimble <modmaker-at-google....@ffmpeg.org> wrote:
> From 1508d19e9f7acf43d76010ce54d59ff204613601 Mon Sep 17 00:00:00 2001
> From: Jacob Trimble <modma...@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.
>
> Signed-off-by: Jacob Trimble <modma...@google.com>
> ---
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 0000000000..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 types.)
> +{
> + AVEncryptionInfo *info;
> +
> + info = av_mallocz(sizeof(AVEncryptionInfo));
> + if (!info)
> + return NULL;
> +
> + info->key_id = av_mallocz(key_id_size);
> + info->key_id_size = key_id_size;
> + info->iv = av_mallocz(iv_size);
> + info->iv_size = iv_size;
> + info->subsamples = av_mallocz_array(sizeof(AVSubsampleEncryptionInfo),
> subsample_count);
> + info->subsample_count = subsample_count;
> +
> + // Allow info->subsamples to be NULL if there are no subsamples.
> + if (!info->key_id || !info->iv || (!info->subsamples &&
> subsample_count)) {
> + av_free(info->key_id);
> + av_free(info->iv);
> + av_free(info->subsamples);
Couldn't this just call av_encryption_info_free()?
> + av_freep(&info);
> + }
> +
> + return info;
> +}
> +
> +void av_encryption_info_free(AVEncryptionInfo* info)
> +{
> + if (info) {
> + av_free(info->key_id);
> + av_free(info->iv);
> + av_free(info->subsamples);
> + av_free(info);
> + }
> +}
> +
> +AVEncryptionInfo* av_encryption_info_copy_side_data(const AVPacket* packet)
Could still be called "get" instead of "copy". I get the intention that
this returns a copy of the data, but "copy" still sounds like it's
being copied somewhere else. But I have no strong feelings about it.
> +{
> + AVEncryptionInfo *info;
> + uint8_t *buffer;
> + unsigned int size;
> + uint32_t key_id_size, iv_size, subsample_count, i;
> +
> + buffer = av_packet_get_side_data(packet, AV_PKT_DATA_ENCRYPTION_INFO,
> &size);
> + if (!buffer)
> + return NULL;
> +
> + key_id_size = AV_RB32(buffer + 12);
> + iv_size = AV_RB32(buffer + 16);
> + subsample_count = AV_RB32(buffer + 20);
> +
> + info = av_encryption_info_alloc(subsample_count, key_id_size, iv_size);
> + if (!info)
> + return NULL;
> +
> + info->scheme = AV_RB32(buffer);
> + info->crypt_byte_block = AV_RB32(buffer + 4);
> + info->skip_byte_block = AV_RB32(buffer + 8);
> + memcpy(info->key_id, buffer + 24, key_id_size);
> + memcpy(info->iv, buffer + key_id_size + 24, iv_size);
> +
> + buffer += key_id_size + iv_size + 24;
> + for (i = 0; i < subsample_count; i++) {
> + info->subsamples[i].bytes_of_clear_data = AV_RB32(buffer);
> + info->subsamples[i].bytes_of_protected_data = AV_RB32(buffer + 4);
> + buffer += 8;
> + }
> +
> + return info;
> +}
> +
> +int av_encryption_info_add_side_data(AVPacket* packet, const
> AVEncryptionInfo* info) {
We put the "{" on a separate line for functions.
> + uint8_t *buffer;
> + size_t size;
> + uint32_t i;
> +
> + size = FF_ENCRYPTION_INFO_EXTRA + info->key_id_size + info->iv_size +
> + (sizeof(AVSubsampleEncryptionInfo) * info->subsample_count);
> + buffer = av_packet_new_side_data(packet, AV_PKT_DATA_ENCRYPTION_INFO,
> size);
> + if (!buffer)
> + return AVERROR(ENOMEM);
> +
> + AV_WB32(buffer, info->scheme);
> + AV_WB32(buffer + 4, info->crypt_byte_block);
> + AV_WB32(buffer + 8, info->skip_byte_block);
> + AV_WB32(buffer + 12, info->key_id_size);
> + AV_WB32(buffer + 16, info->iv_size);
> + AV_WB32(buffer + 20, info->subsample_count);
> + buffer += 24;
> + memcpy(buffer, info->key_id, info->key_id_size);
> + buffer += info->key_id_size;
> + memcpy(buffer, info->iv, info->iv_size);
> + buffer += info->iv_size;
> + for (i = 0; i < info->subsample_count; i++) {
> + AV_WB32(buffer, info->subsamples[i].bytes_of_clear_data);
> + AV_WB32(buffer + 4, info->subsamples[i].bytes_of_protected_data);
> + buffer += 8;
> + }
> +
> + return 0;
> +}
> diff --git a/libavcodec/encryption_info.h b/libavcodec/encryption_info.h
> new file mode 100644
> index 0000000000..4fc9b6d3f1
> --- /dev/null
> +++ b/libavcodec/encryption_info.h
> @@ -0,0 +1,121 @@
> +/**
> + * 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
> + */
> +
> +#ifndef AVUTIL_ENCRYPTION_INFO_H
> +#define AVUTIL_ENCRYPTION_INFO_H
> +
> +#include "avcodec.h"
> +
> +typedef struct AVSubsampleEncryptionInfo {
> + /** 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;
> +} AVSubsampleEncryptionInfo;
> +
> +/**
> + * This describes encryption info for a packet. This contains frame-specific
> + * info for how to decrypt the packet before passing it to the decoder.
> + *
> + * The size of this struct is not part of the public ABI.
> + */
> +typedef struct AVEncryptionInfo {
> + /** The fourcc encryption scheme. */
> + uint32_t scheme;
> +
> + /**
> + * Only used for pattern encryption. This is the number of 16-byte
> blocks
> + * that are encrypted.
> + */
> + uint32_t crypt_byte_block;
> +
> + /**
> + * Only used for pattern encryption. This is the number of 16-byte
> blocks
> + * that are clear.
> + */
> + uint32_t skip_byte_block;
> +
> + /**
> + * The ID of the key used to encrypt the packet. This should always be
> + * 16 bytes long, but may be changed in the future.
> + */
> + uint8_t *key_id;
> + uint32_t key_id_size;
> +
> + /**
> + * The initialization vector. This may have been zero-filled to be the
> + * correct block size. This should always be 16 bytes long, but may be
> + * changed in the future.
> + */
> + uint8_t *iv;
> + uint32_t iv_size;
> +
> + /**
> + * An array of subsample encryption info specifying how parts of the
> sample
> + * are encrypted. If there are no subsamples, then the whole sample is
> + * encrypted.
> + */
> + AVSubsampleEncryptionInfo* subsamples;
> + uint32_t subsample_count;
> +} AVEncryptionInfo;
> +
> +/**
> + * Allocates an AVEncryptionInfo structure and sub-pointers to hold the given
> + * number of subsamples. This will allocate pointers for the key ID, IV,
> + * and subsample entries, set the size members, and zero-initialize the rest.
> + *
> + * @param subsample_count The number of subsamples.
> + * @param key_id_size The number of bytes in the key ID, should be 16.
> + * @param key_id_size The number of bytes in the IV, should be 16.
> + *
> + * @return The new AVEncryptionInfo structure, or NULL on error.
> + */
> +AVEncryptionInfo* av_encryption_info_alloc(uint32_t subsample_count,
> uint32_t key_id_size, uint32_t iv_size);
> +
> +/**
> + * Frees the given encryption info object. This MUST NOT be used to free the
> + * side-data data pointer, that should use normal side-data methods.
> + */
> +void av_encryption_info_free(AVEncryptionInfo* info);
> +
> +/**
> + * Creates a copy of the AVEncryptionInfo that is contained in the side data
> of
> + * the given packet. The resulting object should be passed to
> + * av_encryption_info_free() when done.
> + *
> + * This returns NULL if there is a memory error or if the packet isn't
> encrypted.
> + * To detect if the packet is encrypted, use av_packet_get_side_data.
> + *
> + * @return The new AVEncryptionInfo structure, or NULL on error.
> + */
> +AVEncryptionInfo* av_encryption_info_copy_side_data(const AVPacket* packet);
> +
> +/**
> + * Adds a new side data to the given packet that holds a copy of the given
> + * encryption info.
> + *
> + * @return 0 on success, or a negative error code on error.
> + */
> +int av_encryption_info_add_side_data(AVPacket* packet, const
> AVEncryptionInfo* info);
> +
> +#endif /* AVUTIL_ENCRYPTION_INFO_H */
So as far as the side data management goes, this should be good. No
comment about the crypto stuff or the byte array (de)serialization
stuff, didn't take a close look at it. The final patch should also
include a minor libavutil bump and an doc/APIchanges entry,
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
