[FFmpeg-devel] [PATCH 2/3] lavd: utilize video device callbacks from AVFormatContext

[Nicolas George]

Le quartidi 24 nivôse, an CCXXII, Lukasz Marek a écrit :
> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> ---
>  libavdevice/avdevice.c | 19 +++++++++++++++++++
>  libavdevice/avdevice.h | 16 ++++++++++++++++
>  libavdevice/version.h  |  2 +-
>  3 files changed, 36 insertions(+), 1 deletion(-)

Just a quick suggestion, no time to make a proper review until a few days,
and I do not want to delay things.

> +/**
> + * Call callback to get provided window dimensions.
> + * @param s device context.
> + * @param width window width
> + * @param height window height
> + * @return 0 on success, negative otherwise.
> + */
> +int avdevice_external_window_size(struct AVFormatContext *s, int *width, int *height);
> +
> +/**
> + * Call callback to swap buffers of provided window.
> + * @param s device context.
> + * @return 0 on success, negative otherwise.
> + */
> +int avdevice_swap_external_window_buffers(struct AVFormatContext *s);

The avdevice_external_window_size() API looks strange to me: it seems it is
used to report changes from the application to the device, and therefore
should go in the other direction.

After some thought, I can suggest the following API:

AVFormatContext.control_message(AVFormatContext *s, int type,
                                void *data, size_t data_size);

-> used to plug application-specific callbacks at various events. For
example:

    ctx->control_message(ctx, AV_CTL_MESSAGE_PREPARE_WINDOW_BUFFER, NULL, 0);
    ... OpenGL operations...
    ctx->control_message(ctx, AV_CTL_MESSAGE_DISPLAY_WINDOW_BUFFER, NULL, 0);

AVOutputFormat.control_message(AVFormatContext *s, int type,
                               void *data, size_t data_size);

-> used to signal an output context of various external events. In this
particular case:

    if (SDL_EVENT_WINDOW_SIZE_CHANGE) {
        AVDeviceWindowChange size = {
            .width = new_width,
            .height = new_height
        };
        avformat_control_message(ctx, AV_DEVICE_WINDOW_CHANGE,
                                 &size, sizeof(size));
    }

The thing is that preparing the context and swapping the buffers must be
done by the application, so a custom callback in the context is required,
while reacting to a window size change depends only on the device, so a
method in the format is correct.

But maybe I missed some points I have not thought of.

Also, I believe adding the fields and adding the stubs to call them (i.e.
patches 1 and 2) could be squashed together.

Regards,

-- 
  Nicolas George