[FFmpeg-devel] [PATCH] add fieldorder video filter

[Stefano Sabatini]

On date Saturday 2011-04-09 12:44:28 +0100, Mark Himsley encoded:
> On 08/04/11 19:28, Mark Himsley wrote:
> >On 08/04/11 16:36, Stefano Sabatini wrote:
> >>On date Friday 2011-04-08 12:10:32 +0100, Mark Himsley encoded:
> >>>On 31/03/11 08:47, Mark Himsley wrote:
> >>>>Converting to and from interlaced PAL DV files, with their
> >>>>bottom-field-first interlace field order, can be a pain. Converting tff
> >>>>files to DV results in tff DV files, which are hard to work with in
> >>>>editing software.
> >
> >Thanks for your comments Stefano.
> >
> >[...]
> >
> > >> + const char *tff = "tff";
> > >> + const char *bff = "bff";
> > >
> > > Here you can skip a confusing level of indirection, by directly using
> > > "tff" and "bff" below.
> >
> >I used "tff" and "bff" twice in that function so I though it would be
> >better to define them as const char *s and reuse them.
> >
> >[...]
> >
> >>>+static void start_frame(AVFilterLink *inlink, AVFilterBufferRef
> >>>*inpicref)
> >>>+{
> >>>+ AVFilterContext *ctx = inlink->dst;
> >>>+ FieldOrderContext *fieldorder = ctx->priv;
> >>>+ AVFilterLink *outlink = ctx->outputs[0];
> >>>+
> >>>+ int plane;
> >>>+ AVFilterBufferRef *outpicref;
> >>>+
> >>>+ /** only one time, full an array with the number of bytes that the
> >>>video
> >>>+ * data occupies per line for each plane of the input video */
> >>>+ if(!fieldorder->line_size_set) {
> >>
> >>Nit: if_(...)
> >>
> >>
> >>>+ for (plane = 0; plane< 4; plane++) {
> >>>+ fieldorder->line_size[plane] = av_image_get_linesize(
> >>>+ inlink->format,
> >>>+ inpicref->video->w,
> >>>+ plane);
> >>>+ }
> >>>+ fieldorder->line_size_set = 1;
> >>>+ }
> >>
> >>And I'd prefer to put this in config_props, this way you don't need
> >>line_size_set and the code is slightly more readable.
> >
> >I tried this code in config_props but inlink->cur_buf is null in
> >config_props, where as it isn't in start_frame.
> >
> >But since you've mentioned it, I see that I should be using inlink->w
> >instead; so, yes, it should be in config_props and line_size_set is not
> >needed.
> >
> >>Nit+++:
> >
> >Sorry about all those. Dyslexia and swapping between preferred standards
> >for Java and C are not very good excuses for my poor proof reading.
> 
> Updated patch. I hope I've got all the nit's, but I did leave the
> const char *'s.

Whatever, I can live with it.

I'll commit it tomorrow (with lavfi bump and changelog entry), if
someone doesn't beat me at it.

Thanks :).
-- 
FFmpeg = Fundamental and Frightening Multimedia Pitiless Extravagant Gargoyle