[FFmpeg-devel] [PATCH]Add Dirac support to ffmpeg via libdirac_* and Schroedinger libraries]

Michael Niedermayer michaelni
Thu Apr 10 15:33:58 CEST 2008


On Thu, Apr 10, 2008 at 05:56:21PM +1000, Anuradha Suraparaju wrote:
> 
> On Sat, 2008-04-05 at 01:44 +0200, Michael Niedermayer wrote:
> 
> > [...]
> > > diff --exclude=.svn -ruN ffmpegsvn_trunk/libavcodec/avcodec.h ffmpegsvn_trunk_dirac_schro/libavcodec/avcodec.h
> > > --- ffmpegsvn_trunk/libavcodec/avcodec.h	2008-04-01 09:14:03.000000000 +1000
> > > +++ ffmpegsvn_trunk_dirac_schro/libavcodec/avcodec.h	2008-04-01 09:15:43.000000000 +1000
> > > @@ -182,6 +182,8 @@
> > >      CODEC_ID_8SVX_EXP,
> > >      CODEC_ID_8SVX_FIB,
> > >      CODEC_ID_ESCAPE124,
> > > +    CODEC_ID_SCHRO = 0xFFFE,
> > > +    CODEC_ID_DIRAC = 0xFFFF,
> > 
> > There should only be 1 codec_id and it should not have such a funny number.
> > Also spliting dirac/schroedinger/common demuxer/parser/decoder/encoder in
> > seperate patches would simplify review and likely reduce the number of
> > review-patch-resend iteration needed to get the stuff in svn.
> > 
> > 
> 
> I've attached the dirac patch to this email. It does not include
> dirac_parser.c as the GSoC code can be used without any changes. I've
> created the patch using the latest svn revision 12780. I'll be mailing
> the rest of the patches as soon as possible. 
[...]

> +/**
> +* returns Ffmppeg chroma format
                ^^
typo


> +*/
> +static int GetFfmpegChromaFormat(AVCodecContext *avccontext)
> +{
> +    FfmpegDiracDecoderParams* p_dirac_params = avccontext->priv_data;
> +    dirac_chroma_t dirac_pix_fmt = p_dirac_params->p_decoder->src_params.chroma;
> +
> +    int num_formats = sizeof(ffmpeg_dirac_pixel_format_map)/sizeof(ffmpeg_dirac_pixel_format_map[0]);
> +    int idx;
> +
> +    for (idx = 0; idx < num_formats; ++idx) {
> +        if (ffmpeg_dirac_pixel_format_map[0].dirac_pix_fmt == dirac_pix_fmt) {
                                             ^
I guess that was supposed to be idx ...


> +            avccontext->pix_fmt =  ffmpeg_dirac_pixel_format_map[0].ff_pix_fmt;
> +            return 0;
> +        }
> +    }
> +    av_log (avccontext, AV_LOG_ERROR, "Dirac chroma format %d not supported currntly\n", dirac_pix_fmt);
> +    return -1;
> +}

I think that the following would be clearer:

static PixelFormat GetFfmpegChromaFormat(dirac_chroma_t dirac_pix_fmt)
{
    int num_formats = sizeof(ffmpeg_dirac_pixel_format_map)/sizeof(ffmpeg_dirac_pixel_format_map[0]);
    int idx;

    for (idx = 0; idx < num_formats; ++idx) {
        if (ffmpeg_dirac_pixel_format_map[idx].dirac_pix_fmt == dirac_pix_fmt)
            return ffmpeg_dirac_pixel_format_map[idx].ff_pix_fmt;
        }
    }
    return PIX_FMT_NONE;
}

avccontext->pix_fmt= GetFfmpegChromaFormat(dirac_pix_fmt);
if (avccontext->pix_fmt == PIX_FMT_NONE){
    av_log (avccontext, AV_LOG_ERROR, "Dirac chroma format %d not supported currntly\n", dirac_pix_fmt);
    return -1;
}


[...]
> +static int libdirac_decode_frame(AVCodecContext *avccontext,
> +                              void *data, int *data_size,
> +                              uint8_t *buf, int buf_size)

buf should be const uint8_t *


[...]
> +    while (1) {
> +         /* parse data and process result */
> +        DecoderState state = dirac_parse (p_dirac_params->p_decoder);
> +        switch (state)
> +        {
> +        case STATE_BUFFER:
> +            return buf_size;
> +
> +        case STATE_SEQUENCE:

> +            /* tell ffmpeg about sequence details*/
> +            avccontext->height = p_dirac_params->p_decoder->src_params.height;
> +            avccontext->width  = p_dirac_params->p_decoder->src_params.width;

width/height should be checked by avcodec_check_dimensions() before using
them.


[...]
> +typedef struct
> +{
> +    int width;
> +    int height;
> +    int frame_rate_num;
> +    int frame_rate_denom;
> +    dirac_encoder_presets_t dirac_video_format;
> +} FfmpegDiracSchroVideoFormatInfo;
> +
> +static const FfmpegDiracSchroVideoFormatInfo ffmpeg_dirac_video_format_info[] = {
> +    { 640,  480,  24000, 1001, VIDEO_FORMAT_CUSTOM           },
> +    { 176,  120,  15000, 1001, VIDEO_FORMAT_QSIF525          },
> +    { 176,  144,  25,    2,    VIDEO_FORMAT_QCIF             },
> +    { 352,  240,  15000, 1001, VIDEO_FORMAT_SIF525           },
> +    { 352,  288,  25,    2,    VIDEO_FORMAT_CIF              },
> +    { 704,  480,  15000, 1001, VIDEO_FORMAT_4SIF525          },
> +    { 704,  576,  25,    2,    VIDEO_FORMAT_4CIF             },
> +    { 720,  480,  30000, 1001, VIDEO_FORMAT_SD_480I60        },
> +    { 720,  576,  25,    1,    VIDEO_FORMAT_SD_576I50        },
> +    { 1280, 720,  60000, 1001, VIDEO_FORMAT_HD_720P60        },
> +    { 1280, 720,  50,    1,    VIDEO_FORMAT_HD_720P50        },
> +    { 1920, 1080, 30000, 1001, VIDEO_FORMAT_HD_1080I60       },
> +    { 1920, 1080, 25,    1,    VIDEO_FORMAT_HD_1080I50       },
> +    { 1920, 1080, 60000, 1001, VIDEO_FORMAT_HD_1080P60       },
> +    { 1920, 1080, 50,    1,    VIDEO_FORMAT_HD_1080P50       },
> +    { 2048, 1080, 24,    1,    VIDEO_FORMAT_DIGI_CINEMA_2K24 },
> +    { 4096, 2160, 24,    1,    VIDEO_FORMAT_DIGI_CINEMA_4K24 },
> +};

this could be:

in diracschro_common.h
typedef struct
{
    uint16_t width;
    uint16_t height;
    uint16_t frame_rate_num;
    uint16_t frame_rate_denom;
} FfmpegDiracSchroVideoFormatInfo;

in diracschro_common.c:
const FfmpegDiracSchroVideoFormatInfo ff_dirac_schro_video_format_info[] = {
    { 640,  480,  24000, 1001},
    { 176,  120,  15000, 1001},
    { 176,  144,  25,    2   },
    { 352,  240,  15000, 1001},
    { 352,  288,  25,    2   },
    { 704,  480,  15000, 1001},
    { 704,  576,  25,    2   },
    { 720,  480,  30000, 1001},
    { 720,  576,  25,    1   },
    { 1280, 720,  60000, 1001},
    { 1280, 720,  50,    1   },
    { 1920, 1080, 30000, 1001},
    { 1920, 1080, 25,    1   },
    { 1920, 1080, 60000, 1001},
    { 1920, 1080, 50,    1   },
    { 2048, 1080, 24,    1   },
    { 4096, 2160, 24,    1   },
};

in libdirac.c

dirac_encoder_presets_t somename[]={
    VIDEO_FORMAT_CUSTOM           ,
    VIDEO_FORMAT_QSIF525          ,
    VIDEO_FORMAT_QCIF             ,
    VIDEO_FORMAT_SIF525           ,
    VIDEO_FORMAT_CIF              ,
    VIDEO_FORMAT_4SIF525          ,
    VIDEO_FORMAT_4CIF             ,
    VIDEO_FORMAT_SD_480I60        ,
    VIDEO_FORMAT_SD_576I50        ,
    VIDEO_FORMAT_HD_720P60        ,
    VIDEO_FORMAT_HD_720P50        ,
    VIDEO_FORMAT_HD_1080I60       ,
    VIDEO_FORMAT_HD_1080I50       ,
    VIDEO_FORMAT_HD_1080P60       ,
    VIDEO_FORMAT_HD_1080P50       ,
    VIDEO_FORMAT_DIGI_CINEMA_2K24 ,
    VIDEO_FORMAT_DIGI_CINEMA_4K24 ,
};


> +
> +/**
> +* Works out Dirac-compatible pre-set option from file size
> +*/
> +static dirac_encoder_presets_t GetDiracPreset(AVCodecContext *avccontext)
> +{
> +    const FfmpegDiracSchroVideoFormatInfo *ret_vf = NULL;
> +    int idx;
> +    int num_formats = sizeof(ffmpeg_dirac_video_format_info)/sizeof(ffmpeg_dirac_video_format_info[0]);
> +

> +    /* Get the first match for width and height */
> +    for (idx = 1; idx < num_formats; ++idx) {
> +        if (avccontext->width  == ffmpeg_dirac_video_format_info[idx].width &&
> +            avccontext->height == ffmpeg_dirac_video_format_info[idx].height) {
> +           ret_vf = &ffmpeg_dirac_video_format_info[idx];
> +           break;
> +        }
> +    }
> +
> +    /* Now check if there are any other entries for the same width and height
> +       that match the time_base. If one doesn't exist then return the match
> +       obtained for width and height */
> +    for ( ; idx < num_formats && ret_vf != NULL; ++idx ) {
> +        const FfmpegDiracSchroVideoFormatInfo *vf = &ffmpeg_dirac_video_format_info[idx];
> +        if (avccontext->width  == vf->width                 &&
> +            avccontext->height == vf->height                &&
> +            avccontext->time_base.den == vf->frame_rate_num &&
> +            avccontext->time_base.num == vf->frame_rate_denom) {
> +            ret_vf = vf;
> +        }
> +    }
> +    return ret_vf == NULL ? VIDEO_FORMAT_CUSTOM : ret_vf->dirac_video_format ;

That can be simplified to:

for (idx = 1 ; idx < num_formats; ++idx ) {
    const FfmpegDiracSchroVideoFormatInfo *vf = &ffmpeg_dirac_video_format_info[idx];
    if (avccontext->width  == vf->width &&
        avccontext->height == vf->height){
        ret_vf = vf;
        if (avccontext->time_base.den == vf->frame_rate_num &&
            avccontext->time_base.num == vf->frame_rate_denom) {
            return vf;
        }
    }
}
return ret_vf ? ret_vf->dirac_video_format : VIDEO_FORMAT_CUSTOM;



[...]
> +    if (avccontext->flags & CODEC_FLAG_QSCALE) {
> +        if (avccontext->global_quality != 0) {

> +            p_dirac_params->enc_ctx.enc_params.qf=avccontext->global_quality/(float)FF_QP2LAMBDA/10.0;

minor simplification:
p_dirac_params->enc_ctx.enc_params.qf= avccontext->global_quality/(FF_QP2LAMBDA*10);


[...]
> +    while(go) {
> +        p_dirac_params->p_encoder->enc_buf.buffer = frame;
> +        p_dirac_params->p_encoder->enc_buf.size   = buf_size;
> +        /* process frame */
> +        state = dirac_encoder_output ( p_dirac_params->p_encoder );
> +
> +        switch (state)
> +        {
> +        case ENC_STATE_AVAIL:
> +        case ENC_STATE_EOS:
> +            assert (p_dirac_params->p_encoder->enc_buf.size > 0);
> +            /* create output frame*/
> +            p_frame_output = av_mallocz(sizeof(FfmpegDiracEncodedFrame));
> +            /* set output data */
> +            p_frame_output->p_data = av_malloc(p_dirac_params->p_encoder->enc_buf.size);
> +            memcpy(p_frame_output->p_data,p_dirac_params->p_encoder->enc_buf.buffer,p_dirac_params->p_encoder->enc_buf.size);
> +
> +            p_frame_output->size = p_dirac_params->p_encoder->enc_buf.size;
> +
> +            p_frame_output->frame_num = p_dirac_params->p_encoder->enc_pparams.pnum;

> +            if (p_dirac_params->p_encoder->enc_pparams.ptype == INTRA_PICTURE &&
> +                p_dirac_params->p_encoder->enc_pparams.ptype == REFERENCE_PICTURE)
> +                p_frame_output->key_frame = 1;

is INTRA_PICTURE == REFERENCE_PICTURE ? If not this will be if(0) :)


[...]
> +AVCodec libdirac_encoder = {
> +    "libdirac",
> +    CODEC_TYPE_VIDEO,
> +    CODEC_ID_DIRAC,
> +    sizeof(FfmpegDiracEncoderParams),
> +    libdirac_encode_init,
> +    libdirac_encode_frame,
> +    libdirac_encode_close,
> +   .capabilities= CODEC_CAP_DELAY,
> +} ;

The supported pix_fmts should be listed in there as well
.pix_fmts= (enum PixelFormat[]){PIX_FMT_YUV420P, ..., -1},


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- 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-devel/attachments/20080410/f5872113/attachment.pgp>



More information about the ffmpeg-devel mailing list