[FFmpeg-devel] [PATCH] Add new vf_tiltandshift filter

[FFmpeg-devel] [PATCH] Add new vf_tiltandshift filter

[Vittorio Giovara]

[-- Attachment #1: Type: text/plain, Size: 190 bytes --]

This is an older filter I wrote and never got around publishing.
It can be used to generate a distortion effect like
https://vimeo.com/104938599?share=copy
Please see attached.
-- 
Vittorio

[-- Attachment #2: 0001-Add-new-vf_tiltandshift-filter.patch --]
[-- Type: application/octet-stream, Size: 18285 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add new vf_tiltandshift filter

[Nicolas George]

Vittorio Giovara (12023-12-11):
> This is an older filter I wrote and never got around publishing.
> It can be used to generate a distortion effect like
> https://vimeo.com/104938599?share=copy
> Please see attached.

Your code is doing in request_frame() what should be done in
filter_frame(). That will work in simple linear graphs but not in
graphs with branches and/or time shifts. See doc/filter_design.txt for
details.

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add new vf_tiltandshift filter

[Vittorio Giovara]

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]

On Tue, Dec 12, 2023 at 3:00 AM Nicolas George <george@nsup.org> wrote:

> Vittorio Giovara (12023-12-11):
> > This is an older filter I wrote and never got around publishing.
> > It can be used to generate a distortion effect like
> > https://vimeo.com/104938599?share=copy
> > Please see attached.
>
> Your code is doing in request_frame() what should be done in
> filter_frame(). That will work in simple linear graphs but not in
> graphs with branches and/or time shifts. See doc/filter_design.txt for
> details.
>

Thanks for the review, attached the proposed changes.
-- 
Vittorio

[-- Attachment #2: 0001-Add-new-vf_tiltandshift-filter.patch --]
[-- Type: application/octet-stream, Size: 21844 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add new vf_tiltandshift filter

[Andreas Rheinhardt]

Vittorio Giovara:
> +#include "avfilter.h"
> +#include "formats.h"
> +#include "internal.h"
> +#include "video.h"
> +
> +#define TILT_NONE  -1
> +#define TILT_FRAME  0
> +#define TILT_BLACK  1

Why is this not an enum?

> +
> +typedef struct FrameList {
> +    AVFrame *frame;
> +    struct FrameList *next;
> +} FrameList;
> +
> +typedef struct TiltandshiftContext {
> +    const AVClass *class;
> +
> +    /* set when all input frames have been processed and we have to
> +     * empty buffers, pad and then return */
> +    int eof_recv;
> +
> +    /* live or static sliding */
> +    int tilt;
> +
> +    /* initial or final actions to perform (pad/hold a 
> frame/black/nothing) */
> +    int start;
> +    int end;
> +
> +    /* columns to hold or pad at the beginning or at the end 
> (respectively) */
> +    int hold;
> +    int pad;
> +
> +    /* buffers for black columns */
> +    uint8_t *black_buffers[4];
> +    int black_linesizes[4];
> +
> +    /* list containing all input frames */
> +    size_t input_size;
> +    FrameList *input;
> +    FrameList *prev;
> +
> +    const AVPixFmtDescriptor *desc;
> +} TiltandshiftContext;
> +
> +static int list_add_frame(FrameList **list, size_t *size, AVFrame 
> *frame)
> +{
> +    FrameList *element = av_mallocz(sizeof(FrameList));

The overhead of this FrameList is unnecessary: You can simply use
AVFrame.opaque as your next pointer.

> +    if (!element)
> +        return AVERROR(ENOMEM);
> +
> +    element->frame = frame;
> +
> +    if (*list == NULL) {
> +        *list = element;
> +    } else {
> +        FrameList *head = *list;
> +        while (head->next)
> +            head = head->next;
> +        head->next = element;
> +    }
> +
> +    (*size)++;
> +    return 0;
> +}
> +
> +static void list_remove_head(FrameList **list, size_t *size)
> +{
> +    FrameList *head = *list;
> +    if (head) {
> +        av_frame_free(&head->frame);
> +        *list = head->next;
> +        av_freep(&head);
> +    }
> +
> +    (*size)--;
> +}
> +
> +static const enum AVPixelFormat formats_supported[] = {
> +    AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV444P,
> +    AV_PIX_FMT_YUV410P,
> +    AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P,
> +    AV_PIX_FMT_YUVJ440P,
> +    AV_PIX_FMT_NONE
> +};
> +
> +static int query_formats(AVFilterContext *ctx)
> +{
> +    return ff_set_common_formats(ctx, 
> ff_make_format_list(formats_supported));

Unnecessary. Use FILTER_PIXFMTS_ARRAY.


> +
> +AVFilter ff_vf_tiltandshift = {

const AVFilter

> +    .name          = "tiltandshift",
> +    .description   = NULL_IF_CONFIG_SMALL("Generate a tilt-and-shift'd 
> video."),
> +    .priv_size     = sizeof(TiltandshiftContext),
> +    .priv_class    = &tiltandshift_class,
> +    .uninit        = uninit,
> +    FILTER_INPUTS(tiltandshift_inputs),
> +    FILTER_OUTPUTS(tiltandshift_outputs),
> +    FILTER_QUERY_FUNC(query_formats),
> +};

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add new vf_tiltandshift filter

[Vittorio Giovara]

[-- Attachment #1: Type: text/plain, Size: 461 bytes --]

On Fri, Dec 15, 2023 at 12:34 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> > +static int list_add_frame(FrameList **list, size_t *size, AVFrame
> > *frame)
> > +{
> > +    FrameList *element = av_mallocz(sizeof(FrameList));
>
> The overhead of this FrameList is unnecessary: You can simply use
> AVFrame.opaque as your next pointer.
>

Good tip! Attached an edited version, with all the other suggestions too.
-- 
Vittorio

[-- Attachment #2: 0001-Add-new-vf_tiltandshift-filter.patch --]
[-- Type: application/octet-stream, Size: 21413 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add new vf_tiltandshift filter

[Vittorio Giovara]

On Fri, Dec 15, 2023 at 3:12 PM Vittorio Giovara <vittorio.giovara@gmail.com>
wrote:

>
>
> On Fri, Dec 15, 2023 at 12:34 PM Andreas Rheinhardt <
> andreas.rheinhardt@outlook.com> wrote:
>
>> > +static int list_add_frame(FrameList **list, size_t *size, AVFrame
>> > *frame)
>> > +{
>> > +    FrameList *element = av_mallocz(sizeof(FrameList));
>>
>> The overhead of this FrameList is unnecessary: You can simply use
>> AVFrame.opaque as your next pointer.
>>
>
> Good tip! Attached an edited version, with all the other suggestions too.
> --
> Vittorio
>

If there are no more comments, I'll push this today or tomorrow.
Thanks
-- 
Vittorio
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add new vf_tiltandshift filter

[Nicolas George]

[-- Attachment #1.1: Type: text/plain, Size: 847 bytes --]

Vittorio Giovara (12023-12-20):
> If there are no more comments, I'll push this today or tomorrow.

I think the change you made after the last request might go too far, but
I have not had time to look at the code carefully enough to be sure.

If a filter can produce several output frames from one input, then it
must send at lease one of the NEW frames.

An illustration to make it clear: if I1 allows to compute O1a, O1b, O1c,
I2 allows to compute O2a, O2b, O2c, etc.

Then when I1 arrives, the filter must output at least O1a, it can output
O1b and O1c or not.

But when I2 arrives, it must output at lease O2a, which means it must
output O1b and O1c if that was not done at the time of I1.

Your filter only outputs one frame per input, but it seems to me it can
create several frames.

Regards,

-- 
  Nicolas George

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add new vf_tiltandshift filter

[Vittorio Giovara]

On Wed, Dec 20, 2023 at 2:50 PM Nicolas George <george@nsup.org> wrote:

> Vittorio Giovara (12023-12-20):
> > If there are no more comments, I'll push this today or tomorrow.
>
> I think the change you made after the last request might go too far, but
> I have not had time to look at the code carefully enough to be sure.
>
> If a filter can produce several output frames from one input, then it
> must send at lease one of the NEW frames.
>
> An illustration to make it clear: if I1 allows to compute O1a, O1b, O1c,
> I2 allows to compute O2a, O2b, O2c, etc.
>
> Then when I1 arrives, the filter must output at least O1a, it can output
> O1b and O1c or not.
>
> But when I2 arrives, it must output at lease O2a, which means it must
> output O1b and O1c if that was not done at the time of I1.
>
> Your filter only outputs one frame per input, but it seems to me it can
> create several frames.
>

the filter needs at least $width input frames to generate an output frame,
so it queues them
when the buffer is full, it will pop the head and generate the output frame
when the next input frame arrives, but the amount of input frames in the
queue needed to generate new frames is constant so it cannot output any
additional frames
at the end it will flush the buffer and generate the remaining output frames
-- 
Vittorio
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add new vf_tiltandshift filter

[Vittorio Giovara]

On Wed, Dec 20, 2023 at 2:56 PM Vittorio Giovara <vittorio.giovara@gmail.com>
wrote:

>
>
> On Wed, Dec 20, 2023 at 2:50 PM Nicolas George <george@nsup.org> wrote:
>
>> Vittorio Giovara (12023-12-20):
>> > If there are no more comments, I'll push this today or tomorrow.
>>
>> I think the change you made after the last request might go too far, but
>> I have not had time to look at the code carefully enough to be sure.
>>
>> If a filter can produce several output frames from one input, then it
>> must send at lease one of the NEW frames.
>>
>> An illustration to make it clear: if I1 allows to compute O1a, O1b, O1c,
>> I2 allows to compute O2a, O2b, O2c, etc.
>>
>> Then when I1 arrives, the filter must output at least O1a, it can output
>> O1b and O1c or not.
>>
>> But when I2 arrives, it must output at lease O2a, which means it must
>> output O1b and O1c if that was not done at the time of I1.
>>
>> Your filter only outputs one frame per input, but it seems to me it can
>> create several frames.
>>
>
> the filter needs at least $width input frames to generate an output frame,
> so it queues them
> when the buffer is full, it will pop the head and generate the output
> frame when the next input frame arrives, but the amount of input frames in
> the queue needed to generate new frames is constant so it cannot output any
> additional frames
> at the end it will flush the buffer and generate the remaining output
> frames
> --
>

pushed thanks
-- 
Vittorio
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add new vf_tiltandshift filter

[Paul B Mahol]

There are some serious issues in this code.
Also missing tests.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add new vf_tiltandshift filter

[Ronald S. Bultje]

Hi Paul,

On Fri, Dec 22, 2023 at 10:33 AM Paul B Mahol <onemda@gmail.com> wrote:

> There are some serious issues in this code.
>

While already very insightful, it would be enormously helpful if you could
expand on this statement with more detail, such as a list of issues as they
exist in this code.

Ronald
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add new vf_tiltandshift filter

[Paul B Mahol]

On Fri, Dec 22, 2023 at 4:41 PM Ronald S. Bultje <rsbultje@gmail.com> wrote:

> Hi Paul,
>
> On Fri, Dec 22, 2023 at 10:33 AM Paul B Mahol <onemda@gmail.com> wrote:
>
> > There are some serious issues in this code.
> >
>
> While already very insightful, it would be enormously helpful if you could
> expand on this statement with more detail, such as a list of issues as they
> exist in this code.
>

Librempeg.


>
> Ronald
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".