[FFmpeg-soc] libavfilter: movie source filter

[Víctor Paesa]

Hi,

On Jan 26, 2008 5:59 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Sat, Jan 26, 2008 at 04:39:58PM +0100, Víctor Paesa wrote:
> > Hi,
> >
> > Here is attached a source filter to read video files.
> >
> > Could you please review it?
> [...]
> i think unconditionally adding a dependancy on libavformat is a little extreem

I made it dependant on a --enable-avfilter-lavf flag in configure.

> [...]
> > +typedef struct {
> > +    // Filter parameters
> > +    double            seek_point;
>
> no floats please, floats make regression testing harder

Changed to in64_t

> [...]
> > +    if (av_open_input_file(&mv->pFormatCtx, mv->file_name, file_iformat, 0, NULL) != 0) {
> > +        av_log(NULL, AV_LOG_ERROR,
> > +            "movie_init() Failed to av_open_input_file '%s'\n", mv->file_name);
>
> no av_log(NULL
> please find some context!

Found some :)

[...]
> the {} placement is inconsistant

Hopefully now is not.

> [...]
> > +    if(pCodec == NULL) {
>
> if(!pCodec)

Done.

> [...]
> > +    // Hack to correct the wrong frame rates generated by some codecs
> > +    if (mv->pCodecCtx->time_base.den>1000 && mv->pCodecCtx->time_base.num==1)
> > +        mv->pCodecCtx->time_base.num=1000;
>
> sorry this is not ok
> you MUST NOT ever write into any time_base field for demuxing and decoding

Removed.

> [...]
> > +                mv->pic->pts = packet.pts * AV_TIME_BASE *
> > +                                av_q2d(mv->pCodecCtx->time_base);
>
> its the timebase of the AVStream you must use!

Oops, yes, thanks.

I also fixed some cosmetics, and updated the copyright year.

Here is attached the next revision:

Regards,
Víctor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg.libavfilter.movie.2.diff
Type: text/x-patch
Size: 13717 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20080127/1999e9e5/attachment.bin>