Re: [FFmpeg-devel] [PATCH 3/3] hwcontext: Add test for device creation and derivation

[Michael Niedermayer]

On Tue, May 15, 2018 at 11:36:13AM +0200, Hendrik Leppkes wrote:
> On Tue, May 15, 2018 at 11:31 AM, Mark Thompson  wrote:
> >
> > I think that means there should be something gating this test (and any 
> > other hardware tests) - how about a configure option 
> > --(en|dis)able-hw-tests, with default value the same as autodetect?  (I 
> > think that has the right properties?  Or it could just be disabled by 
> > default, but that would probably mean it would never get run even in places 
> > where it should be.)
> >
> 
> I don't think having that flag in configure makes a lot of sense. It
> should probably just be a separate make target and not be included in
> "make fate". Default fate should test our code, not external libraries
> or drivers.
> If someone wants to set this up on their fate box after making sure it
> actually worked at least during setup, they can easily add it as a
> second target to the make invocation - ie. "make fate fate-hw" or
> something like that (and the fate.sh script probably most of us use
> for that could be extended to take that as an option), and both get
> accumulated in one result.

sounds good. 
I would be fine with a configure option too as an alternative but i think
its better if changing it doesnt require redoing configure and the whole
build

thx
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] hwcontext: Add test for device creation and derivation

[Hendrik Leppkes]

On Tue, May 15, 2018 at 11:31 AM, Mark Thompson  wrote:
>
> I think that means there should be something gating this test (and any other 
> hardware tests) - how about a configure option --(en|dis)able-hw-tests, with 
> default value the same as autodetect?  (I think that has the right 
> properties?  Or it could just be disabled by default, but that would probably 
> mean it would never get run even in places where it should be.)
>

I don't think having that flag in configure makes a lot of sense. It
should probably just be a separate make target and not be included in
"make fate". Default fate should test our code, not external libraries
or drivers.
If someone wants to set this up on their fate box after making sure it
actually worked at least during setup, they can easily add it as a
second target to the make invocation - ie. "make fate fate-hw" or
something like that (and the fate.sh script probably most of us use
for that could be extended to take that as an option), and both get
accumulated in one result.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] hwcontext: Add test for device creation and derivation

[Mark Thompson]

On 15/05/18 03:16, Michael Niedermayer wrote:
> On Mon, May 14, 2018 at 10:58:58PM +0100, Mark Thompson wrote:
>> This uses any devices it can find on the host system - on a system with no
>> hardware device support or in builds with no support included it will do
>> nothing and pass.
>> ---
>>  libavutil/Makefile |   1 +
>>  libavutil/tests/hwdevice.c | 234 
>> +
>>  tests/fate/libavutil.mak   |   4 +
>>  tests/ref/fate/hwdevice|   1 +
>>  4 files changed, 240 insertions(+)
>>  create mode 100644 libavutil/tests/hwdevice.c
>>  create mode 100644 tests/ref/fate/hwdevice
> 
> does not work here (over ssh if that matters)
> --- ./tests/ref/fate/hwdevice 2018-05-15 04:12:44.462164238 +0200
> +++ tests/data/fate/hwdevice  2018-05-15 04:13:04.606164152 +0200
> @@ -1 +0,0 @@
> -Passed.
> Test hwdevice failed. Look at tests/data/fate/hwdevice.err for details.
> X Error of failed request:  BadRequest (invalid request code or no such 
> operation)
>   Major opcode of failed request:  154 (DRI2)
>   Minor opcode of failed request:  1 (DRI2Connect)
>   Serial number of failed request:  11
>   Current serial number in output stream:  11
> make: *** [fate-hwdevice] Error 1

In some sense this is success - the test found that some hardware device 
doesn't work in your setup.

This is presumably either libva or some OpenCL ICD running with X forwarding 
enabled?  I'm aware that they can blindly make Xlib calls which end up in the 
default error handling (call abort()) when they fail - I run "DISPLAY= 
./ffmpeg..." when in an X forwarded session to avoid this.

I think that means there should be something gating this test (and any other 
hardware tests) - how about a configure option --(en|dis)able-hw-tests, with 
default value the same as autodetect?  (I think that has the right properties?  
Or it could just be disabled by default, but that would probably mean it would 
never get run even in places where it should be.)

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] hwcontext: Add test for device creation and derivation

[Mark Thompson]

On 15/05/18 03:16, Xiang, Haihao wrote:
> On Mon, 2018-05-14 at 22:58 +0100, Mark Thompson wrote:
>> This uses any devices it can find on the host system - on a system with no
>> hardware device support or in builds with no support included it will do
>> nothing and pass.
>> ---
>>  libavutil/Makefile |   1 +
>>  libavutil/tests/hwdevice.c | 234
>> +
>>  tests/fate/libavutil.mak   |   4 +
>>  tests/ref/fate/hwdevice|   1 +
>>  4 files changed, 240 insertions(+)
>>  create mode 100644 libavutil/tests/hwdevice.c
>>  create mode 100644 tests/ref/fate/hwdevice
>>
>> diff --git a/libavutil/Makefile b/libavutil/Makefile
>> index 4fe470748c..d0632f16a6 100644
>> --- a/libavutil/Makefile
>> +++ b/libavutil/Makefile
>> @@ -206,6 +206,7 @@ TESTPROGS =
>> adler32 \
>>  fifo\
>>  hash\
>>  hmac\
>> +hwdevice\
>>  integer \
>>  imgutils\
>>  lfg \
>> diff --git a/libavutil/tests/hwdevice.c b/libavutil/tests/hwdevice.c
>> new file mode 100644
>> index 00..a14c4be8a4
>> --- /dev/null
>> +++ b/libavutil/tests/hwdevice.c
>> @@ -0,0 +1,234 @@
>> +/*
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
>> USA
>> + */
>> +
>> +#include 
>> +
>> +#include "libavutil/hwcontext.h"
>> +
>> +static int test_derivation(AVBufferRef *src_ref, const char *src_name)
>> +{
>> +enum AVHWDeviceType derived_type;
>> +const char *derived_name;
>> +AVBufferRef *derived_ref, *back_ref;
>> +AVHWDeviceContext *src_dev, *derived_dev;
>> +int err;
>> +
>> +src_dev = (AVHWDeviceContext*)src_ref->data;
>> +
>> +derived_type = AV_HWDEVICE_TYPE_NONE;
>> +while (1) {
>> +derived_type = av_hwdevice_iterate_types(derived_type);
>> +if (derived_type == AV_HWDEVICE_TYPE_NONE)
>> +break;
>> +
>> +derived_name = av_hwdevice_get_type_name(derived_type);
>> +
>> +err = av_hwdevice_ctx_create_derived(_ref, derived_type,
>> + src_ref, 0);
>> +if (err < 0) {
>> +fprintf(stderr, "Unable to derive %s -> %s: %d.\n",
>> +src_name, derived_name, err);
>> +continue;
>> +}
>> +
>> +derived_dev = (AVHWDeviceContext*)derived_ref->data;
>> +if (derived_dev->type != derived_type) {
>> +printf("Device derived as type %d has type %d.\n",
>> +   derived_type, derived_dev->type);
> 
> back_ref might not be initialized in the error handling path,
> 
>> +goto fail;
>> +}
>> +
>> +if (derived_type == src_dev->type) {
>> +if (derived_dev != src_dev) {
>> +printf("Derivation of %s from itself succeeded but did "
>> +   "not return the same device.\n", src_name);
> 
> Same as above. 

Right, added the initialisation to both of them.

>> +goto fail;
>> +}
>> +av_buffer_unref(_ref);
>> +continue;
>> +}
>> +
>> +err = av_hwdevice_ctx_create_derived(_ref, src_dev->type,
>> + derived_ref, 0);
>> +if (err < 0) {
>> +printf("Derivation %s to %s succeeded, but derivation back "
>> +   "again failed: %d.\n", src_name, derived_name, err);
>> +goto fail;
>> +}
>> +
>> +if (back_ref->data != src_ref->data) {
>> +printf("Derivation %s to %s succeeded, but derivation back "
>> +   "again did not return the original device.\n",
>> +   src_name, derived_name);
>> +goto fail;
>> +}
>> +
>> +fprintf(stderr, "Successfully tested derivation %s -> 

Re: [FFmpeg-devel] [PATCH 3/3] hwcontext: Add test for device creation and derivation

[James Almer]

On 5/14/2018 6:58 PM, Mark Thompson wrote:
> This uses any devices it can find on the host system - on a system with no
> hardware device support or in builds with no support included it will do
> nothing and pass.
> ---

I'd rather not have a fate test try to run anything on my GPU. It's
usually driver dependent and can be fragile.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] hwcontext: Add test for device creation and derivation

[Xiang, Haihao]

On Mon, 2018-05-14 at 22:58 +0100, Mark Thompson wrote:
> This uses any devices it can find on the host system - on a system with no
> hardware device support or in builds with no support included it will do
> nothing and pass.
> ---
>  libavutil/Makefile |   1 +
>  libavutil/tests/hwdevice.c | 234
> +
>  tests/fate/libavutil.mak   |   4 +
>  tests/ref/fate/hwdevice|   1 +
>  4 files changed, 240 insertions(+)
>  create mode 100644 libavutil/tests/hwdevice.c
>  create mode 100644 tests/ref/fate/hwdevice
> 
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 4fe470748c..d0632f16a6 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -206,6 +206,7 @@ TESTPROGS =
> adler32 \
>  fifo\
>  hash\
>  hmac\
> +hwdevice\
>  integer \
>  imgutils\
>  lfg \
> diff --git a/libavutil/tests/hwdevice.c b/libavutil/tests/hwdevice.c
> new file mode 100644
> index 00..a14c4be8a4
> --- /dev/null
> +++ b/libavutil/tests/hwdevice.c
> @@ -0,0 +1,234 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +#include 
> +
> +#include "libavutil/hwcontext.h"
> +
> +static int test_derivation(AVBufferRef *src_ref, const char *src_name)
> +{
> +enum AVHWDeviceType derived_type;
> +const char *derived_name;
> +AVBufferRef *derived_ref, *back_ref;
> +AVHWDeviceContext *src_dev, *derived_dev;
> +int err;
> +
> +src_dev = (AVHWDeviceContext*)src_ref->data;
> +
> +derived_type = AV_HWDEVICE_TYPE_NONE;
> +while (1) {
> +derived_type = av_hwdevice_iterate_types(derived_type);
> +if (derived_type == AV_HWDEVICE_TYPE_NONE)
> +break;
> +
> +derived_name = av_hwdevice_get_type_name(derived_type);
> +
> +err = av_hwdevice_ctx_create_derived(_ref, derived_type,
> + src_ref, 0);
> +if (err < 0) {
> +fprintf(stderr, "Unable to derive %s -> %s: %d.\n",
> +src_name, derived_name, err);
> +continue;
> +}
> +
> +derived_dev = (AVHWDeviceContext*)derived_ref->data;
> +if (derived_dev->type != derived_type) {
> +printf("Device derived as type %d has type %d.\n",
> +   derived_type, derived_dev->type);

back_ref might not be initialized in the error handling path,

> +goto fail;
> +}
> +
> +if (derived_type == src_dev->type) {
> +if (derived_dev != src_dev) {
> +printf("Derivation of %s from itself succeeded but did "
> +   "not return the same device.\n", src_name);

Same as above. 

> +goto fail;
> +}
> +av_buffer_unref(_ref);
> +continue;
> +}
> +
> +err = av_hwdevice_ctx_create_derived(_ref, src_dev->type,
> + derived_ref, 0);
> +if (err < 0) {
> +printf("Derivation %s to %s succeeded, but derivation back "
> +   "again failed: %d.\n", src_name, derived_name, err);
> +goto fail;
> +}
> +
> +if (back_ref->data != src_ref->data) {
> +printf("Derivation %s to %s succeeded, but derivation back "
> +   "again did not return the original device.\n",
> +   src_name, derived_name);
> +goto fail;
> +}
> +
> +fprintf(stderr, "Successfully tested derivation %s -> %s.\n",
> +src_name, derived_name);
> +
> +av_buffer_unref(_ref);
> +av_buffer_unref(_ref);
> +}
> +
> +return 0;
> +
> +fail:
> +av_buffer_unref(_ref);
> +

Re: [FFmpeg-devel] [PATCH 3/3] hwcontext: Add test for device creation and derivation

[Michael Niedermayer]

On Mon, May 14, 2018 at 10:58:58PM +0100, Mark Thompson wrote:
> This uses any devices it can find on the host system - on a system with no
> hardware device support or in builds with no support included it will do
> nothing and pass.
> ---
>  libavutil/Makefile |   1 +
>  libavutil/tests/hwdevice.c | 234 
> +
>  tests/fate/libavutil.mak   |   4 +
>  tests/ref/fate/hwdevice|   1 +
>  4 files changed, 240 insertions(+)
>  create mode 100644 libavutil/tests/hwdevice.c
>  create mode 100644 tests/ref/fate/hwdevice

does not work here (over ssh if that matters)
--- ./tests/ref/fate/hwdevice   2018-05-15 04:12:44.462164238 +0200
+++ tests/data/fate/hwdevice2018-05-15 04:13:04.606164152 +0200
@@ -1 +0,0 @@
-Passed.
Test hwdevice failed. Look at tests/data/fate/hwdevice.err for details.
X Error of failed request:  BadRequest (invalid request code or no such 
operation)
  Major opcode of failed request:  154 (DRI2)
  Minor opcode of failed request:  1 (DRI2Connect)
  Serial number of failed request:  11
  Current serial number in output stream:  11
make: *** [fate-hwdevice] Error 1

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 3/3] hwcontext: Add test for device creation and derivation

[Mark Thompson]

This uses any devices it can find on the host system - on a system with no
hardware device support or in builds with no support included it will do
nothing and pass.
---
 libavutil/Makefile |   1 +
 libavutil/tests/hwdevice.c | 234 +
 tests/fate/libavutil.mak   |   4 +
 tests/ref/fate/hwdevice|   1 +
 4 files changed, 240 insertions(+)
 create mode 100644 libavutil/tests/hwdevice.c
 create mode 100644 tests/ref/fate/hwdevice

diff --git a/libavutil/Makefile b/libavutil/Makefile
index 4fe470748c..d0632f16a6 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -206,6 +206,7 @@ TESTPROGS = adler32 
\
 fifo\
 hash\
 hmac\
+hwdevice\
 integer \
 imgutils\
 lfg \
diff --git a/libavutil/tests/hwdevice.c b/libavutil/tests/hwdevice.c
new file mode 100644
index 00..a14c4be8a4
--- /dev/null
+++ b/libavutil/tests/hwdevice.c
@@ -0,0 +1,234 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include 
+
+#include "libavutil/hwcontext.h"
+
+static int test_derivation(AVBufferRef *src_ref, const char *src_name)
+{
+enum AVHWDeviceType derived_type;
+const char *derived_name;
+AVBufferRef *derived_ref, *back_ref;
+AVHWDeviceContext *src_dev, *derived_dev;
+int err;
+
+src_dev = (AVHWDeviceContext*)src_ref->data;
+
+derived_type = AV_HWDEVICE_TYPE_NONE;
+while (1) {
+derived_type = av_hwdevice_iterate_types(derived_type);
+if (derived_type == AV_HWDEVICE_TYPE_NONE)
+break;
+
+derived_name = av_hwdevice_get_type_name(derived_type);
+
+err = av_hwdevice_ctx_create_derived(_ref, derived_type,
+ src_ref, 0);
+if (err < 0) {
+fprintf(stderr, "Unable to derive %s -> %s: %d.\n",
+src_name, derived_name, err);
+continue;
+}
+
+derived_dev = (AVHWDeviceContext*)derived_ref->data;
+if (derived_dev->type != derived_type) {
+printf("Device derived as type %d has type %d.\n",
+   derived_type, derived_dev->type);
+goto fail;
+}
+
+if (derived_type == src_dev->type) {
+if (derived_dev != src_dev) {
+printf("Derivation of %s from itself succeeded but did "
+   "not return the same device.\n", src_name);
+goto fail;
+}
+av_buffer_unref(_ref);
+continue;
+}
+
+err = av_hwdevice_ctx_create_derived(_ref, src_dev->type,
+ derived_ref, 0);
+if (err < 0) {
+printf("Derivation %s to %s succeeded, but derivation back "
+   "again failed: %d.\n", src_name, derived_name, err);
+goto fail;
+}
+
+if (back_ref->data != src_ref->data) {
+printf("Derivation %s to %s succeeded, but derivation back "
+   "again did not return the original device.\n",
+   src_name, derived_name);
+goto fail;
+}
+
+fprintf(stderr, "Successfully tested derivation %s -> %s.\n",
+src_name, derived_name);
+
+av_buffer_unref(_ref);
+av_buffer_unref(_ref);
+}
+
+return 0;
+
+fail:
+av_buffer_unref(_ref);
+av_buffer_unref(_ref);
+return -1;
+}
+
+static int test_device(enum AVHWDeviceType type, const char *name,
+   const char *device, AVDictionary *opts, int flags)
+{
+AVBufferRef *ref;
+AVHWDeviceContext *dev;
+int err;
+
+err = av_hwdevice_ctx_create(, type, device, opts, flags);
+if (err < 0) {
+fprintf(stderr, "Failed to create %s device: