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

[Mark Himsley]

On 31/03/11 15:15, Stefano Sabatini wrote:
> On date Thursday 2011-03-31 08:47:38 +0100, Mark Himsley encoded:
>> Second version of this filter. Renamed and updated from the
>> suggestions given for the first version.
>>
>>
>> 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.
>>
>> The attached filter can:
>>
>> Convert field order by either moving all of the lines in the picture
>> up by 1 line (bff to tff conversion) or down by 1 line (tff to bff
>> conversion). The remaining line, the bottom line in bff to tff
>> transforms or the top line in tff to bff transforms, is filled by
>> copying the closest line in that field.
>>
>> Previous to this filter I have used a filter chain like this to do
>> bff to tff conversion.
>>
>> format=yuv422p,crop=720:575:0:1,pad=720:576:0:0:black
>>
>> but that chain does not fill the remaining line.

[...]

>> +static int config_input(AVFilterLink *inlink)
>> +{
>> +    AVFilterContext   *ctx        = inlink->dst;
>> +    FieldOrderContext *fieldorder = ctx->priv;
>> +
>> +    int is_packed_rgba;
>> +    int plane, component;
>> +
>> +    /** discover if the pixel format is packed or planar */
>> +    is_packed_rgba = 1;
>
>> +    for (component = 0; component<  av_pix_fmt_descriptors[inlink->format].nb_components; component++) {
>> +        if (av_pix_fmt_descriptors[inlink->format].comp[component].plane != 0) {
>> +            is_packed_rgba = 0;
>> +        }
>> +    }
>
> Nit, technically this is not necessarily packed RGBA, for example
> we support some YUV packed formats, e.g. uyvy and uyyvyy.

True. Changed locally.

>> +
>> +    /** discover the bytes per pixel for each plane */
>> +    if (is_packed_rgba) {
>> +        fieldorder->line_step[0] = av_get_bits_per_pixel(&av_pix_fmt_descriptors[inlink->format])>>3;
>> +    } else {
>> +        for (plane = 0; plane<  4; plane++) {
>> +            fieldorder->line_step[plane] = 1 + (av_pix_fmt_descriptors[inlink->format].comp->depth_minus1>>  3 );
>> +        }
>> +    }
>
> Check av_image_fill_max_pixsteps() in libavutil/imgutils.h, I suspect
> that you can use it in place of the above code.

I don't get the same values with av_image_fill_max_pixsteps() as I do 
with this code for the "weird" packed formats...

>> +    fieldorder->hsub = av_pix_fmt_descriptors[inlink->format].log2_chroma_w;
>> +
>> +    return 0;
>> +}
>> +
>
>> +static void draw_slice(AVFilterLink *link, int y, int h, int slice_dir)
>> +{
>> +    /** cannot do slices because the filter has
>> +     *  to read from pixels outside of the slice */
>> +}
>
> Nit++: call it null_draw_slice() { }, this is the convention used in
> other similar filters.
>
>> +static void end_frame(AVFilterLink *inlink)
>> +{
>> +    AVFilterContext   *ctx        = inlink->dst;
>> +    FieldOrderContext *fieldorder = ctx->priv;
>> +    AVFilterLink      *outlink    = ctx->outputs[0];
>> +
>> +    AVFilterBufferRef *inpic      = inlink->cur_buf;
>> +    AVFilterBufferRef *outpic     = outlink->out_buf;
>> +
>> +    int plane, line, linesize, linewidth, hsub, h, w;
>> +    uint8_t *cpy_src, *cpy_dst;
>> +
>> +    h = outpic->video->h;
>> +    w = outpic->video->w;
>> +
>> +    if (inpic->video->interlaced) {
>> +        if (inpic->video->top_field_first != fieldorder->dst_tff) {
>> +            av_dlog(ctx,
>> +                    "picture will move %s one line\n",
>> +                    fieldorder->dst_tff ? "up" : "down");
>> +            for (plane = 0; plane<  4&&  outpic->data[plane]; plane++) {
>> +                linesize = outpic->linesize[plane];
>> +                hsub = plane == 1 || plane == 2 ? fieldorder->hsub : 0;
>
>> +                linewidth = (w>>  hsub) * fieldorder->line_step[plane];
>
> This unfortunately can't work with "weird" packed formats, like UYVY,
> UYYVYY etc (packed formats with subsampled components).

... which means that this code does work with the "weird" packed formats 
yuyv422, uyvy422.

Scale cannot convert between my source, which is yuv420p, and uyyvyy411 
or y400a, so I have not been able to test those.


> You can add a condition for skipping them in query_formats, or simply
> enumerate the working formats, whatever you prefer.
>
>> +                cpy_src = inpic->data[plane];
>> +                cpy_dst = outpic->data[plane];
>> +                if (fieldorder->dst_tff) {
>> +                    /** Move every line up one line, working from
>> +                     *  the top to the bottom of the frame.
>> +                     *  The original top line is lost.
>> +                     *  The new last line is created as a copy of the
>> +                     *  penultimate line from that field. */
>> +                    for (line = 0; line<  h; line++) {
>> +                        if (1 + line<  outpic->video->h) {
>> +                            memcpy(cpy_dst, cpy_src + linesize, linewidth);
>> +                        } else {
>> +                            memcpy(cpy_dst, cpy_src - linesize - linesize, linewidth);
>> +                        }
>> +                        cpy_src += linesize;
>> +                        cpy_dst += linesize;
>> +                    }
>> +                } else {
>> +                    /** Move every line down one line, working from
>> +                     *  the bottom to the top of the frame.
>> +                     *  The original bottom line is lost.
>> +                     *  The new first line is created as a copy of the
>> +                     *  second line from that field. */
>> +                    cpy_src += (h - 1) * linesize;
>> +                    cpy_dst += (h - 1) * linesize;
>> +                    for (line = h - 1; line>= 0 ; line--) {
>> +                        if ( line>  0) {
>> +                            memcpy(cpy_dst, cpy_src - linesize, linewidth);
>> +                        } else {
>> +                            memcpy(cpy_dst, cpy_src + linesize + linesize, linewidth);
>> +                        }
>> +                        cpy_src -= linesize;
>> +                        cpy_dst -= linesize;
>> +                    }
>> +                }
>> +            }
>> +            outpic->video->top_field_first = fieldorder->dst_tff;
>
>> +        } else {
>> +            av_dlog(ctx,
>> +                    "field order already correct\n");
>> +        }
>> +    }
>
> Missing copy.
>
> BTW, if !inpic->video->interlaced you can avoid to copy and use the
> usual draw_slice() mechanism by adopting the technique used here:
> http://thread.gmane.org/gmane.comp.video.ffmpeg.devel/119581/focus=121439
>
> but I'm OK with simply copying the frame here if you believe it looks
> too complicate.

My local version now does draw_slice() if the filter is doing nothing. 
New version when I clean out the debugging av_log()s.

-- 
Mark