Re: [PATCH] Add Dirac support to ffmpeg

Hi

On Thursday 07 July 2005 14:38, Anuradha Suraparaju wrote:
> **Dirac patch for ffmpeg
> **Tested with ffmpeg cvs (as of July 7, 2007 10:00am) and
>   FFMpeg-20050707
> **Author(s): Andrew Kennedy (dirac_dev <at> users.sourceforge.net) (main)
>              Anuradha Suraparaju (asuraparaju <at> users.sourceforge.net)
>              (minor mods)
>
> Dirac is an open-source video codec currently under development. Both
> the encoder and decoder have been integrated with ffmpeg. This patch is
> only a basic integration such that only the salient paremeters are
> currently passed from ffmpeg to Dirac. As Dirac develops, especially
> with the integration of  a formalised bit-stream syntax,  then more
> complex screnarios can be dealt with. Encoded Dirac files, with the
> right patch, can be played back using MPlayer.
[...]

> +dirac_encoder_presets_t GetDiracPreset(AVCodecContext *avccontext)

shouldnt that and a few other functions be static to avoid namespace 
pollution, or are they used outside dirac.c?

> +       default:
> +               av_log (avccontext, AV_LOG_ERROR, "this codec supports only 
YUV420P\n");
> +               return format420;
> +       
> +       }
> +       
> +       return format420;

the last return is unreachable, IMHO it should be removed

it seems GetFrameBufferSize() could be replaced by avpicture_get_size()

> +static unsigned char* dirac_videobuffer = 0;
> +static int dirac_videobuf_len =0;
> +static unsigned char dirac_videobuf_code[5];
> +static int dirac_videobuf_code_len = 0;

non constant static/globals arent acceptable, it should be possible to use 
several demuxers or decoders at once

> +         }
> +
> +           dirac_videobuffer[dirac_videobuf_len++] = byte;

i dont care what indention style is used as long as its not the randomized one

> +    av_free(dirac_videobuffer);
> +    dirac_videobuffer=0;

this could be replaced by av_freep()

and if i understand the demuxing code in drc.c correctly, then it does only 
split the stream into packets, this should be done in a AVParser instead for 
raw "containers", and the remaining (de)muxing code might fit better in 
libavformat/raw.c instead of a separate file, but a separate file is fine too 
if preferred

--

-- 
Michael