From 64bcffc1ec56da76e4fe631cf31fbc091cc49392 Mon Sep 17 00:00:00 2001 From: John Koleszar Date: Thu, 15 Nov 2012 12:19:07 -0800 Subject: [PATCH] Pack invisible frames without lengths Modify the decoder to return the ending position of the bool decoder and use that as the starting position for the next frame. The constant-space algorithm for parsing the appended frame lengths is O(n^2), which is a potential DoS concern if n is unbounded. Revisit the appended lengths for use as partition lengths when multipartition support is added. In addition, this allows decoding of raw streams outside of a container without additional framing information, though it's insufficient to be able to remux said stream into a container. Change-Id: I71e801a9c3e37abe559a56a597635b0cbae1934b --- vp9/common/onyxd.h | 2 +- vp9/decoder/decodframe.c | 9 +++++- vp9/decoder/onyxd_if.c | 5 ++-- vp9/decoder/onyxd_int.h | 2 +- vp9/vp9_cx_iface.c | 61 ++++++++++++++++++---------------------- vp9/vp9_dx_iface.c | 41 ++++++++++++++++++++++----- vpx/src/vpx_decoder.c | 42 ++------------------------- 7 files changed, 77 insertions(+), 85 deletions(-) diff --git a/vp9/common/onyxd.h b/vp9/common/onyxd.h index 110c7535f..7b7662b3c 100644 --- a/vp9/common/onyxd.h +++ b/vp9/common/onyxd.h @@ -42,7 +42,7 @@ extern "C" void vp9_initialize_dec(void); int vp9_receive_compressed_data(VP9D_PTR comp, unsigned long size, - const unsigned char *dest, + const unsigned char **dest, int64_t time_stamp); int vp9_get_raw_frame(VP9D_PTR comp, YV12_BUFFER_CONFIG *sd, diff --git a/vp9/decoder/decodframe.c b/vp9/decoder/decodframe.c index 03a5d3ed6..c6da8553a 100644 --- a/vp9/decoder/decodframe.c +++ b/vp9/decoder/decodframe.c @@ -985,7 +985,7 @@ static void read_coef_probs(VP9D_COMP *pbi, BOOL_DECODER* const bc) { } } -int vp9_decode_frame(VP9D_COMP *pbi) { +int vp9_decode_frame(VP9D_COMP *pbi, const unsigned char **p_data_end) { BOOL_DECODER header_bc, residual_bc; VP9_COMMON *const pc = &pbi->common; MACROBLOCKD *const xd = &pbi->mb; @@ -1422,5 +1422,12 @@ int vp9_decode_frame(VP9D_COMP *pbi) { #endif // printf("Frame %d Done\n", frame_count++); + /* Find the end of the coded buffer */ + while (residual_bc.count > CHAR_BIT + && residual_bc.count < VP9_BD_VALUE_SIZE) { + residual_bc.count -= CHAR_BIT; + residual_bc.user_buffer--; + } + *p_data_end = residual_bc.user_buffer; return 0; } diff --git a/vp9/decoder/onyxd_if.c b/vp9/decoder/onyxd_if.c index c9f5820a0..863127090 100644 --- a/vp9/decoder/onyxd_if.c +++ b/vp9/decoder/onyxd_if.c @@ -315,13 +315,14 @@ static int swap_frame_buffers(VP9_COMMON *cm) { } int vp9_receive_compressed_data(VP9D_PTR ptr, unsigned long size, - const unsigned char *source, + const unsigned char **psource, int64_t time_stamp) { #if HAVE_ARMV7 int64_t dx_store_reg[8]; #endif VP9D_COMP *pbi = (VP9D_COMP *) ptr; VP9_COMMON *cm = &pbi->common; + const unsigned char *source = *psource; int retcode = 0; /*if(pbi->ready_for_new_data == 0) @@ -380,7 +381,7 @@ int vp9_receive_compressed_data(VP9D_PTR ptr, unsigned long size, pbi->common.error.setjmp = 1; - retcode = vp9_decode_frame(pbi); + retcode = vp9_decode_frame(pbi, psource); if (retcode < 0) { #if HAVE_ARMV7 diff --git a/vp9/decoder/onyxd_int.h b/vp9/decoder/onyxd_int.h index 2684c0448..cbb13ffe8 100644 --- a/vp9/decoder/onyxd_int.h +++ b/vp9/decoder/onyxd_int.h @@ -83,7 +83,7 @@ typedef struct VP9Decompressor { } VP9D_COMP; -int vp9_decode_frame(VP9D_COMP *cpi); +int vp9_decode_frame(VP9D_COMP *cpi, const unsigned char **p_data_end); #if CONFIG_DEBUG diff --git a/vp9/vp9_cx_iface.c b/vp9/vp9_cx_iface.c index f15531902..ae60ae122 100644 --- a/vp9/vp9_cx_iface.c +++ b/vp9/vp9_cx_iface.c @@ -77,8 +77,8 @@ struct vpx_codec_alg_priv { VP9_PTR cpi; unsigned char *cx_data; unsigned int cx_data_sz; - unsigned char *altref_cx_data; - unsigned int altref_size; + unsigned char *pending_cx_data; + unsigned int pending_cx_data_sz; vpx_image_t preview_img; unsigned int next_frame_flag; vp8_postproc_cfg_t preview_ppcfg; @@ -577,19 +577,6 @@ static void pick_quickcompress_mode(vpx_codec_alg_priv_t *ctx, } } -static void append_length(unsigned char* cx_data, unsigned long int *cx_size) { - unsigned char chunk; - unsigned int offset = 0; - unsigned long int size = *cx_size; - do { - chunk = size & 0x7F; - size >>= 7; - chunk |= (offset == 0) << 7; - cx_data[offset] = chunk; - offset++; - } while (size); - *cx_size += offset; -} static vpx_codec_err_t vp8e_encode(vpx_codec_alg_priv_t *ctx, const vpx_image_t *img, @@ -693,14 +680,24 @@ static vpx_codec_err_t vp8e_encode(vpx_codec_alg_priv_t *ctx, ctx->next_frame_flag = 0; } + cx_data = ctx->cx_data; + cx_data_sz = ctx->cx_data_sz; lib_flags = 0; - if (ctx->altref_size) { - cx_data = ctx->altref_cx_data + ctx->altref_size; - cx_data_sz = ctx->cx_data_sz - ctx->altref_size; - } else { - cx_data = ctx->cx_data; - cx_data_sz = ctx->cx_data_sz; + /* Any pending invisible frames? */ + if (ctx->pending_cx_data) { + memmove(cx_data, ctx->pending_cx_data, ctx->pending_cx_data_sz); + ctx->pending_cx_data = cx_data; + cx_data += ctx->pending_cx_data_sz; + cx_data_sz -= ctx->pending_cx_data_sz; + + /* TODO: this is a minimal check, the underlying codec doesn't respect + * the buffer size anyway. + */ + if (cx_data_sz < ctx->cx_data_sz / 2) { + ctx->base.err_detail = "Compressed data buffer too small"; + return VPX_CODEC_ERROR; + } } while (cx_data_sz >= ctx->cx_data_sz / 2 && @@ -712,13 +709,11 @@ static vpx_codec_err_t vp8e_encode(vpx_codec_alg_priv_t *ctx, vpx_codec_cx_pkt_t pkt; VP9_COMP *cpi = (VP9_COMP *)ctx->cpi; - /* TODO(jkoleszar): for now we append lengths to all frames, revisit - * this later to ensure if this is necessary */ - append_length(cx_data + size, &size); - + /* Pack invisible frames with the next visisble frame */ if (!cpi->common.show_frame) { - ctx->altref_cx_data = cx_data; - ctx->altref_size = size; + if (!ctx->pending_cx_data) + ctx->pending_cx_data = cx_data; + ctx->pending_cx_data_sz += size; cx_data += size; cx_data_sz -= size; continue; @@ -777,14 +772,14 @@ static vpx_codec_err_t vp8e_encode(vpx_codec_alg_priv_t *ctx, } else*/ { - if (ctx->altref_size) { - pkt.data.frame.sz = ctx->altref_size + size; - pkt.data.frame.buf = ctx->altref_cx_data; - ctx->altref_size = 0; - ctx->altref_cx_data = NULL; + if (ctx->pending_cx_data) { + pkt.data.frame.buf = ctx->pending_cx_data; + pkt.data.frame.sz = ctx->pending_cx_data_sz + size; + ctx->pending_cx_data = NULL; + ctx->pending_cx_data_sz = 0; } else { pkt.data.frame.buf = cx_data; - pkt.data.frame.sz = size; + pkt.data.frame.sz = size; } pkt.data.frame.partition_id = -1; vpx_codec_pkt_list_add(&ctx->pkt_list.head, &pkt); diff --git a/vp9/vp9_dx_iface.c b/vp9/vp9_dx_iface.c index c85b423b2..74321560f 100644 --- a/vp9/vp9_dx_iface.c +++ b/vp9/vp9_dx_iface.c @@ -303,11 +303,11 @@ static void yuvconfig2image(vpx_image_t *img, img->self_allocd = 0; } -static vpx_codec_err_t vp8_decode(vpx_codec_alg_priv_t *ctx, - const uint8_t *data, - unsigned int data_sz, - void *user_priv, - long deadline) { +static vpx_codec_err_t decode_one(vpx_codec_alg_priv_t *ctx, + const uint8_t **data, + unsigned int data_sz, + void *user_priv, + long deadline) { vpx_codec_err_t res = VPX_CODEC_OK; ctx->img_avail = 0; @@ -317,7 +317,7 @@ static vpx_codec_err_t vp8_decode(vpx_codec_alg_priv_t *ctx, * of the heap. */ if (!ctx->si.h) - res = ctx->base.iface->dec.peek_si(data, data_sz, &ctx->si); + res = ctx->base.iface->dec.peek_si(*data, data_sz, &ctx->si); /* Perform deferred allocations, if required */ @@ -424,6 +424,33 @@ static vpx_codec_err_t vp8_decode(vpx_codec_alg_priv_t *ctx, return res; } +static vpx_codec_err_t vp9_decode(vpx_codec_alg_priv_t *ctx, + const uint8_t *data, + unsigned int data_sz, + void *user_priv, + long deadline) { + const uint8_t *data_start = data; + const uint8_t *data_end = data + data_sz; + vpx_codec_err_t res; + + do { + res = decode_one(ctx, &data_start, data_sz, user_priv, deadline); + assert(data_start >= data); + assert(data_start <= data_end); + + /* Early exit if there was a decode error */ + if (res) + break; + + /* Account for suboptimal termination by the encoder. */ + while (data_start < data_end && *data_start == 0) + data_start++; + + data_sz = data_end - data_start; + } while (data_start < data_end); + return res; +} + static vpx_image_t *vp8_get_frame(vpx_codec_alg_priv_t *ctx, vpx_codec_iter_t *iter) { vpx_image_t *img = NULL; @@ -672,7 +699,7 @@ CODEC_INTERFACE(vpx_codec_vp9_dx) = { { vp8_peek_si, /* vpx_codec_peek_si_fn_t peek_si; */ vp8_get_si, /* vpx_codec_get_si_fn_t get_si; */ - vp8_decode, /* vpx_codec_decode_fn_t decode; */ + vp9_decode, /* vpx_codec_decode_fn_t decode; */ vp8_get_frame, /* vpx_codec_frame_get_fn_t frame_get; */ }, { diff --git a/vpx/src/vpx_decoder.c b/vpx/src/vpx_decoder.c index 4398d927e..1f575e0a0 100644 --- a/vpx/src/vpx_decoder.c +++ b/vpx/src/vpx_decoder.c @@ -109,29 +109,6 @@ vpx_codec_err_t vpx_codec_get_stream_info(vpx_codec_ctx_t *ctx, return SAVE_STATUS(ctx, res); } -static int read_frame_length(const uint8_t *data, uint64_t size, - uint64_t *frame_length, int *size_length) { - uint64_t value = 0; - *size_length = 0; - do { - uint64_t index; - size -= value + *size_length; - index = size - 1; - value = 0; - do { - if (data + index < data) { - *frame_length = -1; - return -1; - } - value <<= 7; - value |= (data[index] & 0x7F); - } while (!(data[index--] >> 7)); - *size_length = size - 1 - index; - } while (value + *size_length < size); - *frame_length = value; - return 0; -} - vpx_codec_err_t vpx_codec_decode(vpx_codec_ctx_t *ctx, const uint8_t *data, @@ -139,11 +116,6 @@ vpx_codec_err_t vpx_codec_decode(vpx_codec_ctx_t *ctx, void *user_priv, long deadline) { vpx_codec_err_t res; - int offset = 0; - uint64_t length = 0; - unsigned char altref_frame; - unsigned int cx_size = data_sz; - uint8_t *cx_data = data; /* Sanity checks */ /* NULL data ptr allowed if data_sz is 0 too */ @@ -152,18 +124,8 @@ vpx_codec_err_t vpx_codec_decode(vpx_codec_ctx_t *ctx, else if (!ctx->iface || !ctx->priv) res = VPX_CODEC_ERROR; else { - do { - altref_frame = !(*cx_data & 0x10); - res = read_frame_length(cx_data, cx_size, &length, &offset); - if (res != 0) - return SAVE_STATUS(ctx, VPX_CODEC_UNSUP_BITSTREAM); - res = ctx->iface->dec.decode(ctx->priv->alg_priv, cx_data, - length, user_priv, deadline); - if (res != 0) - return SAVE_STATUS(ctx, res); - cx_data += offset + length; - cx_size -= offset + length; - } while (cx_data - data <= data_sz && altref_frame); + res = ctx->iface->dec.decode(ctx->priv->alg_priv, data, data_sz, + user_priv, deadline); } return SAVE_STATUS(ctx, res);