Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add derive-device function which searches for existing devices in both directions #25

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

softworkz
Copy link
Collaborator

@softworkz softworkz commented Apr 30, 2022

This is an updated version of:
[PATCH v4 1/1] avutils/hwcontext: When deriving a hwdevice, search for
existing device in both directions

There has been an objection that the earlier patchset would change
API behavior, and that this change should be limited to ffmpeg cli.

To achieve this, the API behavior is left unchanged now and a new
function av_hwdevice_ctx_get_or_create_derived() is added and
used by the hwupload and hwmap filters.

v2: Implemented concept for "weak references" to avoid circular
reference lockup.

v3: rebased due to conflicts

cc: Mark Thompson sw@jkqxz.net
cc: Soft Works softworkz@hotmail.com

@softworkz
Copy link
Collaborator Author

/submit

@ffmpeg-codebot
Copy link

Submitted as pull.25.ffstaging.FFmpeg.1651349262.ffmpegagent@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-25/softworkz/derive_devices-v1

To fetch this version to local tag pr-ffstaging-25/softworkz/derive_devices-v1:

git fetch --no-tags https://github.com/ffstaging/FFmpeg tag pr-ffstaging-25/softworkz/derive_devices-v1

libavutil/hwcontext.c Outdated Show resolved Hide resolved
@ffmpeg-codebot
Copy link

User Mark Thompson <sw@jkqxz.net> has been added to the cc: list.

@ffmpeg-codebot
Copy link

User Soft Works <softworkz@hotmail.com> has been added to the cc: list.

@softworkz
Copy link
Collaborator Author

/submit

@ffmpeg-codebot
Copy link

Submitted as pull.25.v2.ffstaging.FFmpeg.1653142062.ffmpegagent@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-25/softworkz/derive_devices-v2

To fetch this version to local tag pr-ffstaging-25/softworkz/derive_devices-v2:

git fetch --no-tags https://github.com/ffstaging/FFmpeg tag pr-ffstaging-25/softworkz/derive_devices-v2

This allows to create AVBufferRef from AVBuffer directly.

Signed-off-by: softworkz <softworkz@hotmail.com>
…ting devices in both directions

The test /libavutil/tests/hwdevice checks that when deriving a device
from a source device and then deriving back to the type of the source
device, the result is matching the original source device, i.e. the
derivation mechanism doesn't create a new device in this case.

Previously, this test was usually passed, but only due to two different
kind of flaws:

1. The test covers only a single level of derivation (and back)

It derives device Y from device X and then Y back to the type of X and
checks whether the result matches X.

What it doesn't check for, are longer chains of derivation like:

CUDA1 > OpenCL2 > CUDA3 and then back to OpenCL4

In that case, the second derivation returns the first device (CUDA3 ==
CUDA1), but when deriving OpenCL4, hwcontext.c was creating a new
OpenCL4 context instead of returning OpenCL2, because there was no link
from CUDA1 to OpenCL2 (only backwards from OpenCL2 to CUDA1)

If the test would check for two levels of derivation, it would have
failed.

This patch fixes those (yet untested) cases by introducing forward
references (derived_device) in addition to the existing back references
(source_device).

2. hwcontext_qsv didn't properly set the source_device

In case of QSV, hwcontext_qsv creates a source context internally
(vaapi, dxva2 or d3d11va) without calling av_hwdevice_ctx_create_derived
and without setting source_device.

This way, the hwcontext test ran successful, but what practically
happened, was that - for example - deriving vaapi from qsv didn't return
the original underlying vaapi device and a new one was created instead:
Exactly what the test is intended to detect and prevent. It just
couldn't do so, because the original device was hidden (= not set as the
source_device of the QSV device).

This patch properly makes these setting and fixes all derivation
scenarios.

(at a later stage, /libavutil/tests/hwdevice should be extended to check
longer derivation chains as well)

Signed-off-by: softworkz <softworkz@hotmail.com>
…_ctx_get_or_create_derived()

Signed-off-by: softworkz <softworkz@hotmail.com>
…d method

Signed-off-by: softworkz <softworkz@hotmail.com>
@softworkz
Copy link
Collaborator Author

/submit

@ffmpeg-codebot
Copy link

Submitted as pull.25.v3.ffstaging.FFmpeg.1658446783.ffmpegagent@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-25/softworkz/derive_devices-v3

To fetch this version to local tag pr-ffstaging-25/softworkz/derive_devices-v3:

git fetch --no-tags https://github.com/ffstaging/FFmpeg tag pr-ffstaging-25/softworkz/derive_devices-v3

@ffmpeg-codebot
Copy link

On the FFmpeg mailing list, Soft Works wrote (reply to this):



> -----Original Message-----
> From: ffmpegagent <ffmpegagent@gmail.com>
> Sent: Friday, July 22, 2022 1:40 AM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Mark Thompson <sw@jkqxz.net>; Soft Works <softworkz@hotmail.com>;
> softworkz <softworkz@hotmail.com>
> Subject: [PATCH v3 0/4] Add derive-device function which searches for
> existing devices in both directions
> 
> This is an updated version of: [PATCH v4 1/1] avutils/hwcontext: When
> deriving a hwdevice, search for existing device in both directions
> 
> There has been an objection that the earlier patchset would change
> API
> behavior, and that this change should be limited to ffmpeg cli.
> 
> To achieve this, the API behavior is left unchanged now and a new
> function
> av_hwdevice_ctx_get_or_create_derived() is added and used by the
> hwupload
> and hwmap filters.
> 
> v2: Implemented concept for "weak references" to avoid circular
> reference
> lockup.
> 
> v3: rebased due to conflicts
> 
> softworkz (4):
>   avutil/buffer: add av_ref_from_buffer() function
>   avutils/hwcontext: add derive-device function which searches for
>     existing devices in both directions
>   lavu: bump minor version and add doc/APIchanges entry for
>     av_hwdevice_ctx_get_or_create_derived()
>   avfilter/hwmap,hwupload: use new
> av_hwdevice_ctx_get_or_create_derived
>     method

Hi,

@Mark Thompson - would you mind to take a look at this?

You were the only one who was objecting and I had reworked 
this in the sense of our discussion (hopefully).

Thanks,
softworkz


_______________________________________________
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".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant