[FFmpeg-devel] [PATCH] mpeg4video: Add support for MPEG-4 Simple Studio Profile.

Michael Niedermayer michael at niedermayer.cc
Mon Jan 1 21:01:53 EET 2018


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 <kieran at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180101/d1b7e87b/attachment.sig>


More information about the ffmpeg-devel mailing list