[FFmpeg-cvslog] r15010 - in trunk: Changelog libavcodec/dv.c libavcodec/dvdata.h libavformat/dv.c libavformat/isom.c

Michael Niedermayer michaelni
Fri Sep 5 20:45:13 CEST 2008


On Fri, Aug 29, 2008 at 12:41:00AM +0200, romansh wrote:
> Author: romansh
> Date: Fri Aug 29 00:41:00 2008
> New Revision: 15010
> 
> Log:
> Intial implementation of the DV100 (AKA DVCPRO HD) decoder and demuxer as
> specified in SMPTE 370M

[...]

> Modified: trunk/libavcodec/dv.c
> ==============================================================================
> --- trunk/libavcodec/dv.c	(original)
> +++ trunk/libavcodec/dv.c	Fri Aug 29 00:41:00 2008
[...]

> @@ -59,8 +64,8 @@ typedef struct DVVideoContext {
>  
>  /* MultiThreading - dv_anchor applies to entire DV codec, not just the avcontext */
>  /* one element is needed for each video segment in a DV frame */
> -/* at most there are 2 DIF channels * 12 DIF sequences * 27 video segments (PAL 50Mbps) */
> -#define DV_ANCHOR_SIZE (2*12*27)
> +/* at most there are 4 DIF channels * 12 DIF sequences * 27 video segments (1080i50) */
> +#define DV_ANCHOR_SIZE (4*12*27)

comment is not doxygen compatible


>  
>  static void* dv_anchor[DV_ANCHOR_SIZE];
>  
> @@ -102,6 +107,17 @@ static void dv_build_unquantize_tables(D
>              }
>          }
>      }
> +
> +    for(a = 0; a < 4; a++) {
> +        for(q = 0; q < 16; q++) {
> +            for(i = 1; i < 64; i++) {
> +                s->dv100_idct_factor[0][a][q][i]= (dv100_qstep[q]<<(a+9))*dv_iweight_1080_y[i];
> +                s->dv100_idct_factor[1][a][q][i]= (dv100_qstep[q]<<(a+9))*dv_iweight_1080_c[i];
> +                s->dv100_idct_factor[2][a][q][i]= (dv100_qstep[q]<<(a+9))*dv_iweight_720_y[i];
> +                s->dv100_idct_factor[3][a][q][i]= (dv100_qstep[q]<<(a+9))*dv_iweight_720_c[i];
> +            }
> +        }
> +    }
>  }
>  

as video is eiher 1080 or 720, it should not be needed to store both.


[...]
> @@ -386,10 +405,17 @@ static inline void dv_decode_video_segme
>              dc = get_sbits(&gb, 9);
>              dct_mode = get_bits1(&gb);
>              class1 = get_bits(&gb, 2);
> +            if (DV_PROFILE_IS_HD(s->sys)) {
> +                mb->idct_put = s->idct_put[0];
> +                mb->scan_table = s->dv_zigzag[0];
> +                mb->factor_table = s->dv100_idct_factor[((s->sys->height == 720)<<1)&(j < 4)][class1][quant];

((s->sys->height == 720)<<1)&(j < 4) is 0


> +                is_field_mode[mb_index] |= !j && dct_mode;

i probably would write
if(!j)
    is_field_mode[mb_index] = dct_mode;
but if you prefer your way ...


> +            } else {
> -            mb->idct_put = s->idct_put[dct_mode && log2_blocksize==3];
> +                mb->idct_put = s->idct_put[dct_mode && log2_blocksize==3];
> -            mb->scan_table = s->dv_zigzag[dct_mode];
> +                mb->scan_table = s->dv_zigzag[dct_mode];
> -            mb->factor_table = s->dv_idct_factor[class1 == 3][dct_mode]
> +                mb->factor_table = s->dv_idct_factor[class1 == 3][dct_mode]
> -                [quant + dv_quant_offset[class1]];
> +                    [quant + dv_quant_offset[class1]];
> +            }
>              dc = dc << 2;
>              /* convert to unsigned because 128 is not added in the
>                 standard IDCT */

> @@ -465,60 +491,54 @@ static inline void dv_decode_video_segme
>          v = *mb_pos_ptr++;
>          mb_x = v & 0xff;
>          mb_y = v >> 8;
> +        /* We work with 720p frames split in half. The odd half-frame (chan==2,3) is displaced :-( */
> +        if (s->sys->height == 720 && ((s->buf[1]>>2)&0x3) == 0) {
> +               mb_y -= (mb_y>17)?18:-72; /* shifting the Y coordinate down by 72/2 macro blocks */
> +        }

I do not understand exactly what this code does, maybe you could elaborate
but it can be merged into the table i think


> +
> +        /* idct_put'ting luminance */
> +        if ((s->sys->pix_fmt == PIX_FMT_YUV420P) ||
> +            (s->sys->pix_fmt == PIX_FMT_YUV411P && mb_x >= (704 / 8)) ||
> +            (s->sys->height >= 720 && mb_y != 134)) {
> +            y_stride = (s->picture.linesize[0]<<((!is_field_mode[mb_index])*log2_blocksize)) - (2<<log2_blocksize);
> +        } else {
> +            y_stride = 0;
> +        }

I think this can be written as
if(mb_x >= s->special_right || mb_y < s->special_bottom)

with the 2 variables calculated in init or the header parsing or part of
the dv_profiles table
and possibly better names than what ive picked here ...


>          y_ptr = s->picture.data[0] + ((mb_y * s->picture.linesize[0] + mb_x)<<log2_blocksize);
> +        for(j = 0; j < 2; j++, y_ptr += y_stride) {
> +            for (i=0; i<2; i++, block += 64, mb++, y_ptr += (1<<log2_blocksize))
> +                 if (s->sys->pix_fmt == PIX_FMT_YUV422P && s->sys->width == 720 && i)
> +                     y_ptr -= (1<<log2_blocksize);
> +                 else
> +                     mb->idct_put(y_ptr, s->picture.linesize[0]<<is_field_mode[mb_index], block);
> +        }

linesize= s->picture.linesize[0]<<is_field_mode[mb_index];
mb[0]    ->idct_put(y_ptr                                 , linesize, block + 0*64);
mb[2]    ->idct_put(y_ptr                       + y_stride, linesize, block + 2*64);
if(s->sys->video_stype != 4){
    mb[1]->idct_put(y_ptr + (1<<log2_blocksize) +         , linesize, block + 1*64);
    mb[3]->idct_put(y_ptr + (1<<log2_blocksize) + y_stride, linesize, block + 3*54);
}


> +
> +        /* idct_put'ting chrominance */
>          c_offset = (((mb_y>>(s->sys->pix_fmt == PIX_FMT_YUV420P)) * s->picture.linesize[1] +
>                       (mb_x>>((s->sys->pix_fmt == PIX_FMT_YUV411P)?2:1)))<<log2_blocksize);
[...]
> +        for(j=2; j; j--) {
> +            uint8_t *c_ptr = s->picture.data[j] + c_offset;

> +            if (s->sys->pix_fmt == PIX_FMT_YUV411P && mb_x >= (704 / 8)) {

like above this could be mb_x >= s->special_right or border_right 


> +                  uint64_t aligned_pixels[64/8];
> +                  uint8_t *pixels = (uint8_t*)aligned_pixels;
> +                  uint8_t *c_ptr1, *ptr1;
> +                  int x, y;
> +                  mb->idct_put(pixels, 8, block);
> +                  for(y = 0; y < (1<<log2_blocksize); y++, c_ptr += s->picture.linesize[j], pixels += 8) {
> +                      ptr1= pixels + (1<<(log2_blocksize-1));
> +                      c_ptr1 = c_ptr + (s->picture.linesize[j]<<log2_blocksize);
> +                      for(x=0; x < (1<<(log2_blocksize-1)); x++) {
> +                          c_ptr[x]= pixels[x];
> +                          c_ptr1[x]= ptr1[x];
> +                      }
> +                  }
> +                  block += 64; mb++;
> +            } else {
> +                  y_stride = (mb_y == 134) ? (1<<log2_blocksize) :
> +                                             s->picture.linesize[j]<<((!is_field_mode[mb_index])*log2_blocksize);

> +                  for (i=0; i<(1<<(s->sys->bpm==8)); i++, block += 64, mb++, c_ptr += y_stride)
> +                       mb->idct_put(c_ptr, s->picture.linesize[j]<<is_field_mode[mb_index], block);

following may be faster, forget it if its not:

linesize= s->picture.linesize[j]<<is_field_mode[mb_index]
(mb++)->    idct_put(c_ptr           , linesize, block); block+=64;
if(s->sys->bpm==8){
    (mb++)->idct_put(c_ptr + y_stride, linesize, block); block+=64;
}


[...]
> Modified: trunk/libavcodec/dvdata.h
> ==============================================================================
> --- trunk/libavcodec/dvdata.h	(original)
> +++ trunk/libavcodec/dvdata.h	Fri Aug 29 00:41:00 2008
> @@ -316,6 +316,13 @@ static const uint8_t dv_quant_shifts[22]
>  static const uint8_t dv_quant_offset[4] = { 6, 3, 0, 1 };
>  static const uint8_t dv_quant_areas[4] = { 6, 21, 43, 64 };
>  
> +/* quantization quanta by QNO for DV100 */
> +static const uint8_t dv100_qstep[16] = {
> +    1, /* QNO = 0 and 1 both have no quantization */
> +    1,
> +    2, 3, 4, 5, 6, 7, 8, 16, 18, 20, 22, 24, 28, 52
> +};
> +
>  /* NOTE: I prefer hardcoding the positioning of dv blocks, it is
>     simpler :-) */
>  
> @@ -2444,6 +2451,3583 @@ static const uint16_t dv_place_422_625[2
>   0x0b34, 0x2322, 0x2f46, 0x4710, 0x1758,
>  };
>  
> +static const uint16_t dv_place_1080i60[4*10*27*5] = {

Tables are not reviewed, finding a efficient way to avoid them probably
fits better in a seperate thread.


[...]

> +
> +
>  /* DV25/50 DCT coefficient weights and inverse weights */
>  /* created by dvtables.py */
>  static const int dv_weight_bits = 18;

> @@ -2489,6 +6073,48 @@ static const int dv_iweight_248[64] = {
>   22017, 25191, 24457, 27962, 22733, 24600, 25971, 29642,
>  };
>  
> +/* the "inverse" DV100 weights are actually just the spec weights (zig-zagged) */
> +static const int dv_iweight_1080_y[64] = {

comment isnt doxygen compatible


> +    128, 16, 16, 17, 17, 17, 18, 18,
> +    18, 18, 18, 18, 19, 18, 18, 19,
> +    19, 19, 19, 19, 19, 42, 38, 40,
> +    40, 40, 38, 42, 44, 43, 41, 41,
> +    41, 41, 43, 44, 45, 45, 42, 42,
> +    42, 45, 45, 48, 46, 43, 43, 46,
> +    48, 49, 48, 44, 48, 49, 101, 98,
> +    98, 101, 104, 109, 104, 116, 116, 123,
> +};

can be vertically aligned, but i saw you already did this in the patch on
ffmpeg-dev.


[...]
> @@ -2632,6 +6258,86 @@ static const DVprofile dv_profiles[] = {
>        .audio_min_samples = { 1896, 1742, 1264 }, /* for 48, 44.1 and 32Khz */
>        .audio_samples_dist = { 1920, 1920, 1920, 1920, 1920 },
>        .audio_shuffle = dv_audio_shuffle625,
> +    },
> +    { .dsf = 0,
> +      .video_stype = 0x14,
> +      .frame_size = 480000,        /* SMPTE-370M - 1080i60 100 Mbps */
> +      .difseg_size = 10,           /* also known as "DVCPRO HD" */
> +      .n_difchan = 4,
> +      .frame_rate = 30000,
> +      .ltc_divisor = 30,
> +      .frame_rate_base = 1001,
> +      .height = 1080,
> +      .width = 1280,
> +      .sar = {{1, 1}, {1, 1}},
> +      .video_place = dv_place_1080i60,
> +      .pix_fmt = PIX_FMT_YUV422P,
> +      .bpm = 8,
> +      .block_sizes = block_sizes_dv100,
> +      .audio_stride = 90,
> +      .audio_min_samples = { 1580, 1452, 1053 }, /* for 48, 44.1 and 32Khz */
> +      .audio_samples_dist = { 1600, 1602, 1602, 1602, 1602 }, /* per SMPTE-314M */
> +      .audio_shuffle = dv_audio_shuffle525,

These also could be vertically aligned


[...]
> +    { .dsf = 1,
> +      .video_stype = 0x18,
> +      .frame_size = 288000,        /* SMPTE-370M - 720p50 100 Mbps */
> +      .difseg_size = 12,           /* also known as "DVCPRO HD" */
> +      .n_difchan = 2,

> +      .frame_rate = 50,
> +      .ltc_divisor = 50,
> +      .frame_rate_base = 1,

frame_rate and _base could be in a AVRational, that would likely simplify
some code


[...]
> @@ -2658,11 +6364,15 @@ enum dv_pack_type {
>       dv_unknown_pack  = 0xff,
>  };
>  
> +#define DV_PROFILE_IS_HD(p) ((p)->video_stype & 0x10)
> +#define DV_PROFILE_IS_1080i50(p) (((p)->video_stype == 0x14) && ((p)->dsf == 1))
> +#define DV_PROFILE_IS_720p50(p)  (((p)->video_stype == 0x18) && ((p)->dsf == 1))
> +
>  /* minimum number of bytes to read from a DV stream in order to determine the profile */

>  #define DV_PROFILE_BYTES (6*80) /* 6 DIF blocks */
>  
> -/* largest possible DV frame, in bytes (PAL 50Mbps) */
> -#define DV_MAX_FRAME_SIZE 288000
> +/* largest possible DV frame, in bytes (1080i50) */
> +#define DV_MAX_FRAME_SIZE 576000

not doxygen compatible



>  
>  /* maximum number of blocks per macroblock in any DV format */
>  #define DV_MAX_BPM 8

> @@ -2691,12 +6401,10 @@ static inline const DVprofile* dv_codec_

i do not think this function should be inline


>  {
>      int i;
>  
> -    if (codec->width != 720)
> -        return NULL;
> -
>      for (i=0; i<sizeof(dv_profiles)/sizeof(DVprofile); i++)
> -       if (codec->height == dv_profiles[i].height && codec->pix_fmt == dv_profiles[i].pix_fmt)
> -           return &dv_profiles[i];

> +       if (codec->height == dv_profiles[i].height && codec->pix_fmt == dv_profiles[i].pix_fmt &&
> +           codec->width == dv_profiles[i].width)

the == dv_profiles can be vertically aligned


[...]
> @@ -119,6 +121,10 @@ static int dv_extract_audio(uint8_t* fra
>      size = (sys->audio_min_samples[freq] + smpls) * 4; /* 2ch, 2bytes */
>      half_ch = sys->difseg_size/2;
>  

> +    /* We work with 720p frames split in half, thus even frames have channels 0,1 and odd 2,3 */
> +    ipcm = (sys->height == 720 && ((frame[1]>>2)&0x3) == 0)?2:0;

buf[1]&0xC


> +    pcm = ppcm[ipcm++];
> +
>      /* for each DIF channel */
>      for (chan = 0; chan < sys->n_difchan; chan++) {
>          /* for each DIF segment */

> @@ -126,10 +132,9 @@ static int dv_extract_audio(uint8_t* fra
>              frame += 6 * 80; /* skip DIF segment header */
>              if (quant == 1 && i == half_ch) {
>                  /* next stereo channel (12bit mode only) */
> -                if (!pcm2)
> +                pcm = ppcm[ipcm++];
> +                if (!pcm)
>                      break;
> -                else
> -                    pcm = pcm2;
>              }
>  
>              /* for each AV sequence */

i cannot comment this because this is not correctly implemented
dv_audio_12to16() clearly belongs in a decoder not in the demuxer.


[...]

> @@ -196,6 +201,9 @@ static int dv_extract_audio_info(DVDemux
>      quant = as_pack[4] & 0x07; /* 0 - 16bit linear, 1 - 12bit nonlinear */
>  
>      /* note: ach counts PAIRS of channels (i.e. stereo channels) */
> +    if (stype == 3) {
> +        ach = 4;
> +    } else
>      ach = (stype == 2 || (quant && (freq == 2))) ? 2 : 1;

ach=  ((int[32]){  2,  0,  4,  8})[stype];

if this is wrong then so is the comment for stype


[...]
> @@ -323,10 +332,19 @@ int dv_produce_packet(DVDemuxContext *c,
>      for (i=0; i<c->ach; i++) {
>         c->audio_pkt[i].size = size;
>         c->audio_pkt[i].pts  = c->abytes * 30000*8 / c->ast[i]->codec->bit_rate;
> +       ppcm[i] = c->audio_buf[i];
>      }
> -    dv_extract_audio(buf, c->audio_buf[0], c->audio_buf[1], c->sys);
> +    dv_extract_audio(buf, ppcm, c->sys);
>      c->abytes += size;
>  
> +    /* We work with 720p frames split in half, thus even frames have channels 0,1 and odd 2,3 */

> +    if (c->sys->height == 720) {
> +        if (((buf[1]>>2)&0x3))

buf[1]&0xC




[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20080905/137c85a9/attachment.pgp>



More information about the ffmpeg-cvslog mailing list