[FFmpeg-devel] [PATCH 5/5] lavfi: add tile video filter.

[Nicolas George]

Le decadi 20 ventôse, an CCXX, Stefano Sabatini a écrit :
> Nit: maybe WxH, so in case we need to add more parameters we won't
> have conflicts (e.g. if we add a color option)

Very good point, changed.

> Maybe some comments about why skip_frame is required.

I would have thought the vsync would be the one needing explanations. I
added for both.

> Nit: TileContext for overall consistency

I hate camelCase, and do not like CamelCase very much, but changed.

> Default size? Or tell the user what's wrong in this case.

Changed into "6x5", like ImageMagick's montage default.

> What's the problem with it?

If I use "pad=iw+4:ih+4:2:2:violet" to add a small border between the frames
in the tile, I probably want the empty space in the last tile to be violet
too, but then I need to duplicate the color specification.

> Is that true? I believed the decode API was taking care of it (return
> decoded frames in presentation order)

It does return the frames in presentation order, but to do that, it needs to
acquire memory for them, probably in decoding order. For example, if you
have "I B P", the order will probably be:

- get packet for I;
- acquire buffer for I and decode;
- return I;
- get packet for P;
- acquire buffer for P and decode;
- return no frame;
- get packet for B;
- acquire buffer for B and decode;
- return B;
- get next/flush packet;
- return P.

> Overall nice work,

Thanks. Updated patch coming soon.

>		     I wonder if it would make sense to add a setkey
> filter for making this filter more useful.

What would it do? If you think of the -skip nokey in the example, maybe the
new explanation will make more sense.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120310/ffdc5933/attachment.asc>