[Ffmpeg-devel] [PATCH] Add Dirac support to ffmpeg]

[Michael Niedermayer]

Hi

On Tue, Aug 30, 2005 at 04:25:54PM +0100, Andrew Kennedy wrote:
> Hi,
> 
> I've amended the patch in line with your recommendations. As part of the 
> changes, encoding should now only work with the -2 strict option.

thanks, but it seems i keep finding issues every time i want to apply it
so i have to annoy you again with a list of recommandet changes


[...]
> +   if(avccontext->strict_std_compliance > FF_COMPLIANCE_EXPERIMENTAL){
> +        av_log(avccontext, AV_LOG_ERROR, "this codec is under development, files encoded with it may not be decodable with future versions!!!\n"
> +               "use vstrict=-2 / -strict -2 to use it anyway\n");
> +        return -1;
> +    }
> +   
> +   /* get dirac preset*/
> +   dirac_encoder_presets_t preset = GetDiracPreset(avccontext);

mixing statements and declarations, breaks with gcc <=2.95


[...]
> +    avccontext->frame_size = avpicture_get_size(avccontext->pix_fmt, avccontext->width, avccontext->height);

this is wrong, frame_size is only used for audio codecs, for video its
not used, and if we ever decide to use it for video too it would be the
number of temporal samples like it is in case of audio which for all 
current video codecs would be 1


[...]
> +static int dirac_encode_frame(AVCodecContext *avccontext,
> +                              unsigned char *frame,
> +                              int buf_size, void *data)
[...]
> +    avccontext->coded_frame->pts= 0;    

this obviously is wrong, 2 frames cant have the same pts, why do you
set it at all?


[...]

> +static int dirac_parse_open(AVCodecParserContext *s)
> +{
> +    FfmpegDiracParseContext *pc = s->priv_data;
> +    pc->p_dirac_videobuffer = (unsigned char *)av_malloc(0x100000);

is the code which writes into this ensuring that it doesnt write over the
end?

[...]

-- 
Michael