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

[Anuradha Suraparaju]

I'll be submitting a libdirac patch soon. But have some comments.

On Sat, 2008-04-05 at 01:44 +0200, Michael Niedermayer wrote:


> [...]
> 
> > diff --exclude=.svn -ruN ffmpegsvn_trunk/libavcodec/schroedinger.c ffmpegsvn_trunk_dirac_schro/libavcodec/schroedinger.c
> > --- ffmpegsvn_trunk/libavcodec/schroedinger.c	1970-01-01 10:00:00.000000000 +1000
> > +++ ffmpegsvn_trunk_dirac_schro/libavcodec/schroedinger.c	2008-04-04 11:29:00.000000000 +1000
> [...]
> > +/* contains a single encoded frame returned from Schroedinger */
> > +typedef struct FfmpegSchroOutputEncFrame
> > +{
> > +    /* frame data */
> > +    unsigned char *p_data;
> > +    
> > +    /* frame size */
> > +    int size;
> > +    
> > +    /* key frame flag */
> > +    int key_frame;
> > +    
> > +    /* next frame to be output in sequence */
> > +    struct FfmpegSchroOutputEncFrame *p_next_frame;
> > +    
> > + } FfmpegSchroOutputEncFrame;
> 
> code duplication relative to dirac
> 

I'd rather not have a common file containing code common to Dirac and
Schroedinger because that would mean both libraries will need to be
installed for successful build. So I'd like to keep them separate.

> 
> [...]
> > +/*
> > +* Works out Schro-compatible pre-set option from file size
> > +*/
> > +static SchroVideoFormatEnum GetSchroVideoFormatPreset(AVCodecContext *avccontext)
> > +{
> > +    if(avccontext->width==176 && avccontext->height==120)
> > +        return SCHRO_VIDEO_FORMAT_QSIF;
> > +    else if(avccontext->width==176 && avccontext->height==144)
> > +        return SCHRO_VIDEO_FORMAT_QCIF;
> > +    else if(avccontext->width==352 && avccontext->height==240)
> > +        return SCHRO_VIDEO_FORMAT_SIF;
> > +    else if(avccontext->width==352 && avccontext->height==288)
> > +        return SCHRO_VIDEO_FORMAT_CIF;
> > +    else if(avccontext->width==704 && avccontext->height==480)
> > +        return SCHRO_VIDEO_FORMAT_4SIF;
> > +    else if(avccontext->width==704 && avccontext->height==576)
> > +        return SCHRO_VIDEO_FORMAT_4CIF;
> > +    else if(avccontext->width==720 && avccontext->height==480)
> > +        return SCHRO_VIDEO_FORMAT_SD480I_60;
> > +    else if(avccontext->width==720 && avccontext->height==576)
> > +        return SCHRO_VIDEO_FORMAT_SD576I_50;
> > +    else if(avccontext->height==1280 && avccontext->height==720) {
> > +        if (avccontext->time_base.den == 60000 && 
> > +            avccontext->time_base.num == 1001) {
> > +            return SCHRO_VIDEO_FORMAT_HD720P_60;
> > +        } else {
> > +            return SCHRO_VIDEO_FORMAT_HD720P_50;
> > +        }
> > +    } else if (avccontext->height==1920 && avccontext->width==1080) {
> > +        if (avccontext->time_base.den == 30000 && 
> > +            avccontext->time_base.num == 1001) {
> > +            return SCHRO_VIDEO_FORMAT_HD1080I_60;
> > +        } else if (avccontext->time_base.den == 25 && 
> > +            avccontext->time_base.num == 1) {
> > +            return SCHRO_VIDEO_FORMAT_HD1080I_50;
> > +        } else if (avccontext->time_base.den == 60 && 
> > +            avccontext->time_base.num == 1001) {
> > +            return SCHRO_VIDEO_FORMAT_HD1080P_60;
> > +        } else {
> > +            return SCHRO_VIDEO_FORMAT_HD1080P_50;
> > +        }
> > +    } else if(avccontext->height==2048 && avccontext->width==1080)
> > +        return SCHRO_VIDEO_FORMAT_DC2K_24;
> > +    else if(avccontext->height==4096 && avccontext->width==2160)
> > +        return SCHRO_VIDEO_FORMAT_DC4K_24;
> > +    return SCHRO_VIDEO_FORMAT_CUSTOM;
> > +}
> > +
> > +/*
> > +* Works out Schro-compatible chroma format 
> > +*/
> > +static int GetSchroChromaFormat(AVCodecContext *avccontext)
> > +{
> > +    FfmpegSchroParams* p_schro_params = avccontext->priv_data;
> > + 
> > +    switch(avccontext->pix_fmt)
> > +    {
> > +    case PIX_FMT_YUV420P:
> > +        p_schro_params->format->chroma_format =  SCHRO_CHROMA_420;
> > +        break;
> > +
> > +    case PIX_FMT_YUV422P:
> > +        p_schro_params->format->chroma_format =  SCHRO_CHROMA_422;
> > +        break;
> > +    
> > +    case PIX_FMT_YUV444P:
> > +        p_schro_params->format->chroma_format =  SCHRO_CHROMA_444;
> > +        break;
> > +    
> > +    default:
> > +        av_log (avccontext, AV_LOG_ERROR, "this codec supports only Planar YUV 4:2:0, 4:2:2 and 4:4:4 formats currently\n");
> > +        return -1;
> > +    }
> > +    return 0;
> > +}
> > + 
> > + /*
> > + * returns Ffmppeg chroma format 
> > + */
> > +static int GetFfmpegChromaFormat(SchroChromaFormat schro_chroma)
> > +{
> > +    switch(schro_chroma)
> > +    {
> > +    case SCHRO_CHROMA_420:
> > +        return PIX_FMT_YUV420P;
> > +    case SCHRO_CHROMA_422:
> > +        return PIX_FMT_YUV422P;
> > +    case SCHRO_CHROMA_444:
> > +        return PIX_FMT_YUV444P;
> > +    
> > +    default:
> > +        break;
> > +    }
> > +      
> > +     return PIX_FMT_YUV420P;
> > +}
> 
> That looks quite similar to dirac
> Also theres no need for a forward and backward switch-case.
> A simple table of the ffmpeg+dirac+shchro IDs could be used.
> Similarly for width-heigt-...
> 

See the comment above regarding common code for Dirac and Schroedinger.
I have however included ffmpeg-dirac pix format maps and width-height to
video format maps for Dirac. I will also do the same for Schroedinger.


Regards,
Anuradha