Hi On Fri, Dec 29, 2017 at 03:46:41PM +0000, Kieran Kunhya wrote: > $subj > > I'm not to happy about the s->block2 stuff, there are many ways of trying > to resolve this (e.g union), so review welcome. > > I will add DPCM support in a later (currently unfinished) patch. > > Kieran
sorry for the delay, i was and am a bit busy ...
> h263dec.c | 8
> idctdsp.c | 2
> ituh263dec.c | 9
> mpeg4data.h | 116 +++++++++++
> mpeg4video.h | 17 +
> mpeg4videodec.c | 513
> ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> mpegvideo.c | 21 +-
> mpegvideo.h | 17 +
> x86/idctdsp_init.c | 1
> 9 files changed, 687 insertions(+), 17 deletions(-)
> 039f599da00adef33163ed09587d3bae69b41c33
> 0001-mpeg4video-Add-support-for-MPEG-4-Simple-Studio-Prof.patch
> From 1f11d8a412271aa4e7aca0dd90c2a41d9d4dd5c7 Mon Sep 17 00:00:00 2001
> From: Kieran Kunhya <kie...@kunhya.com>
> Date: Fri, 29 Dec 2017 15:42:14 +0000
> Subject: [PATCH] mpeg4video: Add support for MPEG-4 Simple Studio Profile.
>
> This is a profile supporting > 8-bit video and has a higher quality DCT
> ---
> libavcodec/h263dec.c | 8 +
> libavcodec/idctdsp.c | 2 +-
> libavcodec/ituh263dec.c | 9 +-
> libavcodec/mpeg4data.h | 116 ++++++++++
> libavcodec/mpeg4video.h | 17 ++
> libavcodec/mpeg4videodec.c | 513
> +++++++++++++++++++++++++++++++++++++++++-
> libavcodec/mpegvideo.c | 21 +-
> libavcodec/mpegvideo.h | 17 +-
> libavcodec/x86/idctdsp_init.c | 1 +
> 9 files changed, 687 insertions(+), 17 deletions(-)
>
> diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
> index 5608b63245..647ff0239b 100644
> --- a/libavcodec/h263dec.c
> +++ b/libavcodec/h263dec.c
> @@ -47,6 +47,10 @@
>
> static enum AVPixelFormat h263_get_format(AVCodecContext *avctx)
> {
> + /* MPEG-4 Studio Profile only, not supported by hardware */
> + if (avctx->bits_per_raw_sample > 8)
> + return avctx->pix_fmt;
If its just for studio profile, that should be checked for or assert()ed
If it ever can be true for some other case this commnet is easy to overlook
> +
> if (avctx->codec->id == AV_CODEC_ID_MSS2)
> return AV_PIX_FMT_YUV420P;
>
> @@ -197,6 +201,10 @@ static int decode_slice(MpegEncContext *s)
>
> ff_set_qscale(s, s->qscale);
>
> + if (s->studio_profile) {
> + ff_mpeg4_decode_studio_slice_header(s->avctx->priv_data);
> + }
> +
> if (s->avctx->hwaccel) {
> const uint8_t *start = s->gb.buffer + get_bits_count(&s->gb) / 8;
> ret = s->avctx->hwaccel->decode_slice(s->avctx, start,
> s->gb.buffer_end - start);
> diff --git a/libavcodec/idctdsp.c b/libavcodec/idctdsp.c
> index 1de372d2b9..e5b0be3cb7 100644
> --- a/libavcodec/idctdsp.c
> +++ b/libavcodec/idctdsp.c
> @@ -256,7 +256,7 @@ av_cold void ff_idctdsp_init(IDCTDSPContext *c,
> AVCodecContext *avctx)
> c->perm_type = FF_IDCT_PERM_NONE;
> } else {
> if (avctx->bits_per_raw_sample == 10 || avctx->bits_per_raw_sample
> == 9) {
> - c->idct_put = ff_simple_idct_put_int16_10bit;
> + c->idct_put = ff_simple_idct_put_int32_10bit;
> c->idct_add = ff_simple_idct_add_int16_10bit;
This doesnt look right, is this intended ?
put is 32bit, add is 16bit
[...]
> +/**
> + * Decode the next video packet.
> + * @return <0 if something went wrong
> + */
> +int ff_mpeg4_decode_studio_slice_header(Mpeg4DecContext *ctx)
> +{
> + MpegEncContext *s = &ctx->m;
> + GetBitContext *gb = &s->gb;
> + unsigned vlc_len;
> + uint16_t mb_num;
> +
> + if (get_bits_long(gb, 32) == SLICE_START_CODE) {
> + vlc_len = av_log2(((s->width + 15) / 16) * ((s->height + 15) / 16))
> + 1;
> + mb_num = get_bits(gb, vlc_len);
> +
> + s->mb_x = mb_num % s->mb_width;
> + s->mb_y = mb_num / s->mb_width;
> +
> + if (ctx->shape != BIN_ONLY_SHAPE)
> + s->qscale = get_qscale(s);
> +
> + if (show_bits1(gb)) {
> + skip_bits1(gb); /* slice_extension_flag */
> + skip_bits1(gb); /* intra_slice */
> + skip_bits1(gb); /* slice_VOP_id_enable */
> + skip_bits(gb, 6); /* slice_VOP_id */
> + while (show_bits1(gb)) {
> + skip_bits1(gb); /* extra_bit_slice */
> + skip_bits(gb, 8); /* slice_VOP_id */
> + }
> + }
> + skip_bits1(gb); /* extra_bit_slice */
you can replace the show_bits1 by get_bits1 above and drop the corresponding
skip_bits1
> +
> + reset_studio_dc_predictors(s);
> + }
> +
> + return 0;
> +}
> +
> /**
> * Get the average motion vector for a GMC MB.
> * @param n either 0 for the x component or 1 for y
> @@ -1718,6 +1775,186 @@ end:
> return SLICE_OK;
> }
>
> +static void next_start_code_studio(GetBitContext *gb)
> +{
> + align_get_bits(gb);
> +
> + while (get_bits_left(gb) >= 24 && show_bits_long(gb, 24) != 0x1) {
> + get_bits(gb, 8);
> + }
> +}
this looks like the normal start code search so seperate code should not
be needed
> +
> +/* additional_code, vlc index */
> +static int ac_state_tab[22][2] =
static const uint8_t
> +{
> + {0, 0},
> + {0, 1},
> + {1, 1},
> + {2, 1},
> + {3, 1},
> + {4, 1},
> + {5, 1},
> + {1, 2},
> + {2, 2},
> + {3, 2},
> + {4, 2},
> + {5, 2},
> + {6, 2},
> + {1, 3},
> + {2, 4},
> + {3, 5},
> + {4, 6},
> + {5, 7},
> + {6, 8},
> + {7, 9},
> + {8, 10},
> + {0, 11}
> +};
> +
> +static int mpeg4_decode_studio_block(MpegEncContext *s, int32_t block[64],
> int n)
> +{
> + Mpeg4DecContext *ctx = (Mpeg4DecContext *)s;
> +
> + int cc, dct_dc_size, dct_diff, code, j, idx = 1, group = 0, run = 0,
> + additional_code_len, sign, mismatch;
> + VLC *cur_vlc = &ctx->studio_intra_tab[0];
> + uint8_t *const scantable = s->intra_scantable.permutated;
> + const uint16_t *quant_matrix;
> + uint32_t flc;
> + const int min = -1 * (1 << (s->avctx->bits_per_raw_sample + 6));
> + const int max = ((1 << (s->avctx->bits_per_raw_sample + 6)) - 1);
> +
> + mismatch = 1;
> +
> + memset(block, 0, 64 * sizeof(int32_t));
> +
> + if (n < 4) {
> + cc = 0;
> + dct_dc_size = get_vlc2(&s->gb, ctx->studio_luma_dc.table,
> STUDIO_INTRA_BITS, 2);
> + quant_matrix = s->intra_matrix;
> + }
> + else {
> + cc = ((n - 4) % 2) + 1; /* Table 7-30 */
(n&1) + 1
> + if (ctx->rgb)
> + dct_dc_size = get_vlc2(&s->gb, ctx->studio_luma_dc.table,
> STUDIO_INTRA_BITS, 2);
> + else
> + dct_dc_size = get_vlc2(&s->gb, ctx->studio_chroma_dc.table,
> STUDIO_INTRA_BITS, 2);
> + quant_matrix = s->chroma_intra_matrix;
> + }
> +
> + if (dct_dc_size == 0) {
> + dct_diff = 0;
> + } else {
> + dct_diff = get_xbits(&s->gb, dct_dc_size);
> +
> + if (dct_dc_size > 8)
> + check_marker(s->avctx, &s->gb, "dct_dc_size > 8");
> + }
> +
> + s->studio_dc_val[cc] += dct_diff;
> +
> + if (s->mpeg_quant)
> + block[0] = s->studio_dc_val[cc] * (8 >> s->intra_dc_precision);
> + else
> + block[0] = s->studio_dc_val[cc] * (8 >> s->intra_dc_precision) * (8
> >> s->dct_precision);
> + // FIXME support mpeg_quant for AC coefficients
> +
> + block[0] = av_clip(block[0], min, max);
> + mismatch ^= block[0];
> +
> + /* AC Coefficients */
> + while (1) {
> + group = get_vlc2(&s->gb, cur_vlc->table, STUDIO_INTRA_BITS, 2);
> +
> + additional_code_len = ac_state_tab[group][0];
> + cur_vlc = &ctx->studio_intra_tab[ac_state_tab[group][1]];
> +
> + if (group == 0) {
> + /* End of Block */
> + break;
> + }
> + else if (group >= 1 && group <= 6) {
> + /* Zero run length (Table B.47) */
> + run = 1 << additional_code_len;
> + if (additional_code_len)
> + run += get_bits(&s->gb, additional_code_len);
> + idx += run;
> + }
> + else if (group >= 7 && group <= 12) {
> + /* Zero run length and +/-1 level (Table B.48) */
> + code = get_bits(&s->gb, additional_code_len);
> + sign = code & 1;
> + code >>= 1;
> + run = (1 << (additional_code_len - 1)) + code;
> + idx += run;
> + j = scantable[idx++];
> + block[j] = sign ? 1 : -1;
> + block[j] = ((8 * 2 * block[j] * quant_matrix[j] * s->qscale) >>
> s->dct_precision) / 32;
> + block[j] = av_clip(block[j], min, max);
> + mismatch ^= block[j];
> + }
> + else if (group >= 13 && group <= 20) {
> + /* Level value (Table B.49) */
> + j = scantable[idx++];
> + block[j] = get_xbits(&s->gb, additional_code_len);
> + block[j] = ((8 * 2 * block[j] * quant_matrix[j] * s->qscale) >>
> s->dct_precision) / 32;
> + block[j] = av_clip(block[j], min, max);
> + mismatch ^= block[j];
> + }
> + else if (group == 21) {
> + /* Escape */
> + j = scantable[idx++];
> + additional_code_len = s->avctx->bits_per_raw_sample +
> s->dct_precision + 4;
> + flc = get_bits(&s->gb, additional_code_len);
> + if (flc >> (additional_code_len-1))
> + block[j] = -1 * (( flc ^ ((1 << additional_code_len) -1)) +
> 1);
> + else
> + block[j] = flc;
> +
> + block[j] = ((8 * 2 * block[j] * quant_matrix[j] * s->qscale) >>
> s->dct_precision) / 32;
> + block[j] = av_clip(block[j], min, max);
> + mismatch ^= block[j];
> + }
you can add a "continue" in
else if (group >= 1 && group <= 6) {
and then factor some common code out of the others
[...]
> +/**
> + * Decode the next studio vop header.
> + * @return <0 if something went wrong
> + */
> +static int decode_studio_vop_header(Mpeg4DecContext *ctx, GetBitContext *gb)
> +{
> + MpegEncContext *s = &ctx->m;
> + int i, v;
> +
> + if (get_bits_left(gb) <= 32)
> + return 0;
> +
> + if (get_bits_long(gb, 32) != VOP_STARTCODE)
> + return -1;
> +
> + s->decode_mb = mpeg4_decode_studio_mb;
> +
> + if (decode_smpte_tc(ctx, gb) < 0)
> + return -1;
please use AVERROR* codes instead of just -1
[...]
> +static void decode_studiovisualobject(Mpeg4DecContext *ctx, GetBitContext
> *gb)
> +{
> + uint32_t startcode;
> + MpegEncContext *s = &ctx->m;
> + int visual_object_type, width, height;
> +
> + startcode = get_bits_long(gb, 32);
> +
> + /* StudioVisualObject() */
> + if (startcode == VISUAL_OBJ_STARTCODE) {
> + skip_bits(gb, 4); /* visual_object_verid */
> + visual_object_type = get_bits(gb, 4);
> +
> + next_start_code_studio(gb);
> + extension_and_user_data(s, gb, 1);
> +
> + if (visual_object_type == VOT_VIDEO_ID) {
> + /* StudioVideoObjectLayer */
> + skip_bits_long(gb, 32); /* video_object_start_code */
> + skip_bits_long(gb, 32); /* video_object_layer_start_code */
> + skip_bits1(gb); /* random_accessible_vol */
> + skip_bits(gb, 8); /* video_object_type_indication */
> + skip_bits(gb, 4); /* video_object_layer_verid */
> + ctx->shape = get_bits(gb, 2); /* video_object_layer_shape */
> + skip_bits(gb, 4); /* video_object_layer_shape_extension */
> + skip_bits1(gb); /* progressive_sequence */
> + if (ctx->shape != BIN_ONLY_SHAPE) {
> + ctx->rgb = get_bits1(gb); /* rgb_components */
> + s->chroma_format = get_bits(gb, 2); /* chroma_format */
> + s->avctx->bits_per_raw_sample = get_bits(gb, 4); /*
> bit_depth */
> + if (s->avctx->bits_per_raw_sample == 10) {
> + if (ctx->rgb) {
> + s->avctx->pix_fmt = AV_PIX_FMT_GBRP10;
> + }
> + else {
> + s->avctx->pix_fmt = s->chroma_format == CHROMA_422 ?
> AV_PIX_FMT_YUV422P10 : AV_PIX_FMT_YUV444P10;
> + }
> + }
bits_per_raw_sample and chroma_format should be checked for validity
thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
No great genius has ever existed without some touch of madness. -- Aristotle
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
