On Sun, Jan 10, 2016 at 1:28 AM, Kieran Kunhya <kie...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel