[FFmpeg-devel] [libav-devel] [PATCH 1/2] avcodec: Add Cineform HD Decoder
Vittorio Giovara
vittorio.giovara at gmail.com
Mon Jan 11 18:17:41 CET 2016
On Sun, Jan 10, 2016 at 1:28 AM, Kieran Kunhya <kieran at kunhya.com> wrote:
> ---
> libavcodec/Makefile | 1 +
> libavcodec/allcodecs.c | 1 +
> libavcodec/avcodec.h | 1 +
> libavcodec/cfhd.c | 565 ++++++++++++++++++++++++++++++++++++++++++++++++
> libavcodec/cfhd.h | 99 +++++++++
> libavcodec/cfhddata.c | 470 ++++++++++++++++++++++++++++++++++++++++
> libavcodec/codec_desc.c | 6 +
> 7 files changed, 1143 insertions(+)
> create mode 100644 libavcodec/cfhd.c
> create mode 100644 libavcodec/cfhd.h
> create mode 100644 libavcodec/cfhddata.c
> diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
> new file mode 100644
> index 0000000..dc36889
> --- /dev/null
> +++ b/libavcodec/cfhd.c
> + for (i = 0; i < len; i++) {
> + if( i == 0 )
> + {
> + tmp = (11*low[0*low_stride] - 4*low[1*low_stride] + low[2*low_stride] + 4) >> 3;
> + output[(2*i+0)*out_stride] = (tmp + high[0*high_stride]) >> 1;
> + tmp = ( 5*low[0*low_stride] + 4*low[1*low_stride] - low[2*low_stride] + 4) >> 3;
> + output[(2*i+1)*out_stride] = (tmp - high[0*high_stride]) >> 1;
> + }
> + else if( i == len-1 )
> + {
> + tmp = ( 5*low[i*low_stride] + 4*low[(i-1)*low_stride] - low[(i-2)*low_stride] + 4) >> 3;
> + output[(2*i+0)*out_stride] = (tmp + high[i*high_stride]) >> 1;
> + tmp = (11*low[i*low_stride] - 4*low[(i-1)*low_stride] + low[(i-2)*low_stride] + 4) >> 3;
> + output[(2*i+1)*out_stride] = (tmp - high[i*high_stride]) >> 1;
> + }
> + else
> + {
> + tmp = (low[(i-1)*low_stride] - low[(i+1)*low_stride] + 4) >> 3;
> + output[(2*i+0)*out_stride] = (tmp + low[i*low_stride] + high[i*high_stride]) >> 1;
> + tmp = (low[(i+1)*low_stride] - low[(i-1)*low_stride] + 4) >> 3;
> + output[(2*i+1)*out_stride] = (tmp + low[i*low_stride] - high[i*high_stride]) >> 1;
> + }
> + }
> +}
this formatting will make Diego cry blood
> +static void horiz_filter(int16_t *output, int16_t *low, int16_t *high, int width)
> +{
> + filter(output, 1, low, 1, high, 1, width);
> +}
> +
> +static void vert_filter(int16_t *output, int out_stride, int16_t *low, int low_stride,
> + int16_t *high, int high_stride, int len)
> +{
> + filter(output, out_stride, low, low_stride, high, high_stride, len);
> +}
these might be very good av_always_inline candidates
> +static int cfhd_decode(AVCodecContext *avctx, void *data, int *got_frame,
> + AVPacket *avpkt)
> +{
> + CFHDContext *s = avctx->priv_data;
> + GetByteContext gb;
> + AVFrame *pic = data;
> + int ret = 0, i, j;
> + int16_t *plane[3] = {NULL};
> + int16_t *tmp[3] = {NULL};
> + int16_t *subband[3][10] = {{0}};
> + int16_t *l_h[3][8];
> + int16_t *coeff_data;
> +
> + avcodec_get_chroma_sub_sample(avctx->pix_fmt, &s->chroma_x_shift, &s->chroma_y_shift);
this is a deprecated function, looks like you have to use
av_pix_fmt_get_chroma_sub_sample and check its return value
> + for (i = 0; i < 3; i++) {
> + int width = i ? avctx->width >> s->chroma_x_shift : avctx->width;
> + int height = i ? avctx->height >> s->chroma_y_shift : avctx->height;
> + int stride = FFALIGN(width / 8, 8) * 8;
> + int w8, h8, w4, h4, w2, h2;
> + height = FFALIGN(height / 8, 2) * 8;
> + s->plane[i].width = width;
> + s->plane[i].height = height;
> + s->plane[i].stride = stride;
> +
> + w8 = FFALIGN(s->plane[i].width / 8, 8);
> + h8 = FFALIGN(s->plane[i].height / 8, 2);
> + w4 = w8 * 2;
> + h4 = h8 * 2;
> + w2 = w4 * 2;
> + h2 = h4 * 2;
> +
> + plane[i] = av_malloc(height * stride * sizeof(*plane[i]));
> + tmp[i] = av_malloc(height * stride * sizeof(*tmp[i]));
> + if (!plane[i] || !tmp[i]) {
> + ret = AVERROR(ENOMEM);
> + goto end;
> + }
> +
> + subband[i][0] = plane[i];
> + subband[i][1] = plane[i] + 2 * w8 * h8;
> + subband[i][2] = plane[i] + 1 * w8 * h8;
> + subband[i][3] = plane[i] + 3 * w8 * h8;
> + subband[i][4] = plane[i] + 2 * w4 * h4;
> + subband[i][5] = plane[i] + 1 * w4 * h4;
> + subband[i][6] = plane[i] + 3 * w4 * h4;
> + subband[i][7] = plane[i] + 2 * w2 * h2;
> + subband[i][8] = plane[i] + 1 * w2 * h2;
> + subband[i][9] = plane[i] + 3 * w2 * h2;
> +
> + l_h[i][0] = tmp[i];
> + l_h[i][1] = tmp[i] + 2 * w8 * h8;
> + //l_h[i][2] = ll2;
> + l_h[i][3] = tmp[i];
> + l_h[i][4] = tmp[i] + 2 * w4 * h4;
> + //l_h[i][5] = ll1;
> + l_h[i][6] = tmp[i];
> + l_h[i][7] = tmp[i] + 2 * w2 * h2;
> + }
> +
> + init_frame_defaults(s);
> +
> + if ((ret = ff_get_buffer(avctx, pic, 0)) < 0)
> + return ret;
you are leaking plane and tmp in case of error, this should probably
be `goto end` or (even better) move this call before the for loop
> + bytestream2_init(&gb, avpkt->data, avpkt->size);
> +
> + while (bytestream2_tell(&gb) < avpkt->size) {
> + /* Bit weird but implement the tag parsing as the spec says */
> + uint16_t tagu = bytestream2_get_be16(&gb);
> + int16_t tag = (int16_t)tagu;
> + int8_t tag8 = (int8_t)(tagu >> 8);
> + uint16_t abstag = abs(tag);
> + int8_t abs_tag8 = abs(tag8);
> + uint16_t data = bytestream2_get_be16(&gb);
> + if (abs_tag8 >= 0x60 && abs_tag8 <= 0x6f) {
> + av_log(avctx, AV_LOG_DEBUG, "large len %x \n", ((tagu & 0xff) << 16) | data);
> + } else if (tag == 20) {
> + av_log(avctx, AV_LOG_DEBUG, "Width %"PRIu16" \n", data);
> + avctx->width = data;
> + } else if (tag == 21) {
> + av_log(avctx, AV_LOG_DEBUG, "Height %"PRIu16" \n", data);
> + avctx->height = data;
> + } else if (tag == 101) {
> + av_log(avctx, AV_LOG_DEBUG, "Bits per component: %"PRIu16" \n", data);
> + s->bpc = data;
> + } else if (tag == 12) {
all these tags are a little magical, I would recommend a proper
define/enum which would help readability
> + av_log(avctx, AV_LOG_DEBUG, "Channel Count: %"PRIu16" \n", data);
> + s->channel_cnt = data;
> + if (data != 3) {
> + av_log(avctx, AV_LOG_ERROR, "Channel Count of %"PRIu16" is unsupported\n", data);
> + ret = AVERROR_PATCHWELCOME;
> + break;
> + }
avpriv_report_missing_feature would be more appropriate, same below
> + } else if (tag == 14) {
> + av_log(avctx, AV_LOG_DEBUG, "Subband Count: %"PRIu16" \n", data);
> + if (data != 10) {
> + av_log(avctx, AV_LOG_ERROR, "Subband Count of %"PRIu16" is unsupported\n", data);
> + ret = AVERROR_PATCHWELCOME;
> + break;
> + }
> +end:
> + for (i = 0; i < 3; i++) {
> + av_freep(&plane[i]);
> + av_freep(&tmp[i]);
> + }
> +
> + if (ret < 0)
> + return ret;
> +
> + *got_frame = 1;
> + return avpkt->size;
> +}
this should not return the packet size, but the bytes consumed, so
avpkt->size - bytestream2_get_bytes_left() (or something along this
line)
> +int ff_cfhd_init_vlcs(CFHDContext *s);
> diff --git a/libavcodec/cfhddata.c b/libavcodec/cfhddata.c
> new file mode 100644
> index 0000000..a08bfc7
> --- /dev/null
> +++ b/libavcodec/cfhddata.c
> +av_cold int ff_cfhd_init_vlcs(CFHDContext *s)
> +{
> + int i, j, ret = 0;
> + uint32_t new_cfhd_vlc_bits[NB_VLC_TABLE_18 * 2];
> + uint8_t new_cfhd_vlc_len[NB_VLC_TABLE_18 * 2];
> + uint16_t new_cfhd_vlc_run[NB_VLC_TABLE_18 * 2];
> + int16_t new_cfhd_vlc_level[NB_VLC_TABLE_18 * 2];
> +
> + /** Similar to dv.c, generate signed VLC tables **/
> +
> + /* Table 9 */
> + for (i = 0, j = 0; i < NB_VLC_TABLE_9; i++, j++) {
> + new_cfhd_vlc_bits[j] = table_9_vlc_bits[i];
> + new_cfhd_vlc_len[j] = table_9_vlc_len[i];
> + new_cfhd_vlc_run[j] = table_9_vlc_run[i];
> + new_cfhd_vlc_level[j] = table_9_vlc_level[i];
> +
> + /* Don't include the zero level nor escape bits */
> + if (table_9_vlc_level[i] &&
> + new_cfhd_vlc_bits[j] != table_9_vlc_bits[NB_VLC_TABLE_9-1]) {
> + new_cfhd_vlc_bits[j] <<= 1;
> + new_cfhd_vlc_len[j]++;
> + j++;
> + new_cfhd_vlc_bits[j] = (table_9_vlc_bits[i] << 1) | 1;
> + new_cfhd_vlc_len[j] = table_9_vlc_len[i] + 1;
> + new_cfhd_vlc_run[j] = table_9_vlc_run[i];
> + new_cfhd_vlc_level[j] = -table_9_vlc_level[i];
> + }
> + }
> +
> + ret = init_vlc(&s->vlc_9, VLC_BITS, j, new_cfhd_vlc_len,
> + 1, 1, new_cfhd_vlc_bits, 4, 4, 0);
> + if (ret < 0)
> + goto end;
you are not doing any cleanup, just return ret
> + for (i = 0; i < s->vlc_9.table_size; i++) {
> + int code = s->vlc_9.table[i][0];
> + int len = s->vlc_9.table[i][1];
> + int level, run;
> +
> + if (len < 0) { // more bits needed
> + run = 0;
> + level = code;
> + } else {
> + run = new_cfhd_vlc_run[code];
> + level = new_cfhd_vlc_level[code];
> + }
> + s->table_9_rl_vlc[i].len = len;
> + s->table_9_rl_vlc[i].level = level;
> + s->table_9_rl_vlc[i].run = run;
> + }
> +
> + /* Table 18 */
> + for (i = 0, j = 0; i < NB_VLC_TABLE_18; i++, j++) {
> + new_cfhd_vlc_bits[j] = table_18_vlc_bits[i];
> + new_cfhd_vlc_len[j] = table_18_vlc_len[i];
> + new_cfhd_vlc_run[j] = table_18_vlc_run[i];
> + new_cfhd_vlc_level[j] = table_18_vlc_level[i];
> +
> + /* Don't include the zero level nor escape bits */
> + if (table_18_vlc_level[i] &&
> + new_cfhd_vlc_bits[j] != table_18_vlc_bits[NB_VLC_TABLE_18-1]) {
> + new_cfhd_vlc_bits[j] <<= 1;
> + new_cfhd_vlc_len[j]++;
> + j++;
> + new_cfhd_vlc_bits[j] = (table_18_vlc_bits[i] << 1) | 1;
> + new_cfhd_vlc_len[j] = table_18_vlc_len[i] + 1;
> + new_cfhd_vlc_run[j] = table_18_vlc_run[i];
> + new_cfhd_vlc_level[j] = -table_18_vlc_level[i];
> + }
> + }
> +
> + ret = init_vlc(&s->vlc_18, VLC_BITS, j, new_cfhd_vlc_len,
> + 1, 1, new_cfhd_vlc_bits, 4, 4, 0);
> + if (ret < 0)
> + goto end;
this leaks vlc_9 in case of error
> + av_assert0(s->vlc_18.table_size == 4572);
> +
> + for (i = 0; i < s->vlc_18.table_size; i++) {
> + int code = s->vlc_18.table[i][0];
> + int len = s->vlc_18.table[i][1];
> + int level, run;
> +
> + if (len < 0) { // more bits needed
> + run = 0;
> + level = code;
> + } else {
> + run = new_cfhd_vlc_run[code];
> + level = new_cfhd_vlc_level[code];
> + }
> + s->table_18_rl_vlc[i].len = len;
> + s->table_18_rl_vlc[i].level = level;
> + s->table_18_rl_vlc[i].run = run;
> + }
> +
> +end:
> + return ret;
> +}
Finally I think 2/2 could be squashed into this one, since the decoder
won't be opened without it.
--
Vittorio
More information about the ffmpeg-devel
mailing list