On 20/02/17 05:56, wm4 wrote:
> On Sun, 19 Feb 2017 18:46:25 +0000
> Mark Thompson <[email protected]> wrote:
>
>> Not yet enabled for any device types.
>> ---
>> Makefile | 2 +-
>> avconv.c | 22 ++++
>> avconv.h | 16 +++
>> avconv_hw.c | 391
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> avconv_opt.c | 8 ++
>> 5 files changed, 438 insertions(+), 1 deletion(-)
>> create mode 100644 avconv_hw.c
>>
>> diff --git a/Makefile b/Makefile
>> index 98eb3ab1d..fc661c546 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -81,7 +81,7 @@ ALLAVPROGS = $(AVBASENAMES:%=%$(EXESUF))
>>
>> $(foreach prog,$(AVBASENAMES),$(eval OBJS-$(prog) += cmdutils.o))
>>
>> -OBJS-avconv += avconv_opt.o avconv_filter.o
>> +OBJS-avconv += avconv_opt.o avconv_filter.o avconv_hw.o
>> OBJS-avconv-$(CONFIG_LIBMFX) += avconv_qsv.o
>> OBJS-avconv-$(CONFIG_VAAPI) += avconv_vaapi.o
>> OBJS-avconv-$(CONFIG_VDA) += avconv_vda.o
>> diff --git a/avconv.c b/avconv.c
>> index 5c36761c1..3f5806788 100644
>> --- a/avconv.c
>> +++ b/avconv.c
>> @@ -1704,6 +1704,17 @@ static int init_input_stream(int ist_index, char
>> *error, int error_len)
>>
>> if (!av_dict_get(ist->decoder_opts, "threads", NULL, 0))
>> av_dict_set(&ist->decoder_opts, "threads", "auto", 0);
>> +
>> + ret = hw_device_setup_for_decode(ist);
>> + if (ret < 0) {
>> + char errbuf[128];
>> + av_strerror(ret, errbuf, sizeof(errbuf));
>> + snprintf(error, error_len, "Device setup failed for "
>> + "decoder on input stream #%d:%d : %s",
>> + ist->file_index, ist->st->index, errbuf);
>> + return ret;
>> + }
>> +
>> if ((ret = avcodec_open2(ist->dec_ctx, codec, &ist->decoder_opts))
>> < 0) {
>> char errbuf[128];
>> if (ret == AVERROR_EXPERIMENTAL)
>> @@ -2046,6 +2057,16 @@ static int init_output_stream(OutputStream *ost, char
>> *error, int error_len)
>> ost->enc_ctx->hw_frames_ctx =
>> av_buffer_ref(ost->filter->filter->inputs[0]->hw_frames_ctx);
>> if (!ost->enc_ctx->hw_frames_ctx)
>> return AVERROR(ENOMEM);
>> + } else {
>> + ret = hw_device_setup_for_encode(ost);
>> + if (ret < 0) {
>> + char errbuf[128];
>> + av_strerror(ret, errbuf, sizeof(errbuf));
>> + snprintf(error, error_len, "Device setup failed for "
>> + "encoder on output stream #%d:%d : %s",
>> + ost->file_index, ost->index, errbuf);
>> + return ret;
>> + }
>> }
>>
>> if ((ret = avcodec_open2(ost->enc_ctx, codec, &ost->encoder_opts))
>> < 0) {
>> @@ -2795,6 +2816,7 @@ static int transcode(void)
>> }
>>
>> av_buffer_unref(&hw_device_ctx);
>> + hw_device_free_all();
>>
>> /* finished ! */
>> ret = 0;
>> diff --git a/avconv.h b/avconv.h
>> index 3c3f0ef65..4c87e77b3 100644
>> --- a/avconv.h
>> +++ b/avconv.h
>> @@ -40,6 +40,7 @@
>> #include "libavutil/avutil.h"
>> #include "libavutil/dict.h"
>> #include "libavutil/fifo.h"
>> +#include "libavutil/hwcontext.h"
>> #include "libavutil/pixfmt.h"
>> #include "libavutil/rational.h"
>>
>> @@ -65,6 +66,12 @@ typedef struct HWAccel {
>> enum AVPixelFormat pix_fmt;
>> } HWAccel;
>>
>> +typedef struct HWDevice {
>> + char *name;
>> + enum AVHWDeviceType type;
>> + AVBufferRef *device_ref;
>> +} HWDevice;
>> +
>> /* select an input stream for an output stream */
>> typedef struct StreamMap {
>> int disabled; /* 1 is this mapping is disabled by a negative
>> map */
>> @@ -510,4 +517,13 @@ int qsv_transcode_init(OutputStream *ost);
>> int vaapi_decode_init(AVCodecContext *avctx);
>> int vaapi_device_init(const char *device);
>>
>> +HWDevice *hw_device_get_by_name(const char *name);
>> +int hw_device_init_from_string(const char *arg, HWDevice **dev);
>> +void hw_device_free_all(void);
>> +
>> +int hw_device_setup_for_decode(InputStream *ist);
>> +int hw_device_setup_for_encode(OutputStream *ost);
>> +
>> +int hwaccel_decode_init(AVCodecContext *avctx);
>> +
>> #endif /* AVCONV_H */
>> diff --git a/avconv_hw.c b/avconv_hw.c
>> new file mode 100644
>> index 000000000..dc712c524
>> --- /dev/null
>> +++ b/avconv_hw.c
>> @@ -0,0 +1,391 @@
>> +/*
>> + * This file is part of Libav.
>> + *
>> + * Libav 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.
>> + *
>> + * Libav 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 Libav; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
>> USA
>> + */
>> +
>> +#include <string.h>
>> +
>> +#include "avconv.h"
>> +
>> +static int nb_hw_devices;
>> +static HWDevice **hw_devices;
>> +
>> +typedef struct HWDeviceType {
>> + const char *name;
>> + enum AVHWDeviceType type;
>> + enum HWAccelID hwaccel_id;
>> +} HWDeviceType;
>> +
>> +const HWDeviceType hw_device_types[] = {
>> +};
>> +
>> +static const HWDeviceType *hw_device_get_type_by_name(const char *name)
>> +{
>> + enum AVHWDeviceType type;
>> + int i;
>> + type = av_hwdevice_find_type_by_name(name);
>> + if (type < 0)
>> + return NULL;
>> + for (i = 0; i < FF_ARRAY_ELEMS(hw_device_types); i++) {
>> + if (hw_device_types[i].type == type)
>> + return &hw_device_types[i];
>> + }
>> + return NULL;
>> +}
>> +
>> +static HWDevice *hw_device_get_by_type(enum AVHWDeviceType type)
>> +{
>> + HWDevice *found = NULL;
>> + int i;
>> + for (i = 0; i < nb_hw_devices; i++) {
>> + if (hw_devices[i]->type == type) {
>> + if (found)
>> + return NULL;
>> + found = hw_devices[i];
>> + }
>> + }
>> + return found;
>> +}
>> +
>> +HWDevice *hw_device_get_by_name(const char *name)
>> +{
>> + int i;
>> + for (i = 0; i < nb_hw_devices; i++) {
>> + if (!strcmp(hw_devices[i]->name, name))
>> + return hw_devices[i];
>> + }
>> + return NULL;
>> +}
>
> Odd choice: why make hwcontext in libavutil generic to the point of
> providing a name lookup function in the previous patch, just to
> introduce a table with every device type here again?
The table here is needed for the hwaccel ID lookup. Hmm. With the opposite
direction function also in hwcontext then we can avoid having the
name-as-string twice.
>> +
>> +static HWDevice *hw_device_new(void)
>> +{
>> + int err;
>> + err = av_reallocp_array(&hw_devices, nb_hw_devices + 1,
>> + sizeof(*hw_devices));
>> + if (err) {
>> + nb_hw_devices = 0;
>> + return NULL;
>> + }
>> + hw_devices[nb_hw_devices] = av_mallocz(sizeof(HWDevice));
>> + if (!hw_devices[nb_hw_devices])
>> + return NULL;
>> + return hw_devices[nb_hw_devices++];
>> +}
>
> I'd call it hw_device_add().
Sure.
>> +
>> +int hw_device_init_from_string(const char *arg, HWDevice **dev_out)
>> +{
>> + // "type=name:device,key=value,key2=value2"
>> + // "type:device,key=value,key2=value2"
>> + // -> av_hwdevice_ctx_create()
>> + // "type=name@name"
>> + // "type@name"
>> + // -> av_hwdevice_ctx_create_derived()
>> +
>> + AVDictionary *options = NULL;
>> + char *type_name = NULL, *name = NULL, *device = NULL;
>> + const HWDeviceType *type;
>> + HWDevice *dev, *src;
>> + AVBufferRef *device_ref;
>> + int err;
>> + const char *errmsg, *p, *q;
>> +
>> + for (p = arg; *p && *p != ':' && *p != '=' && *p != '@'; p++);
>
> That's some sick string processing (in a bad way, unfortunately).
>
> The first *p is redundant. I guess it skips until the first character
> that isn't in a set, isn't that what strcspn() normally does?
*p is "not end". But yes, strcspn() would be better here (I never remember
exactly what all the string functions do).
>> +
>> + type_name = av_strndup(arg, p - arg);
>> + if (!type_name) {
>> + err = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>> + type = hw_device_get_type_by_name(type_name);
>> + if (!type) {
>> + errmsg = "unknown device type";
>> + goto invalid;
>> + }
>> +
>> + if (*p == '=') {
>> + for (q = ++p; *q && *q != ':' && *q != '@'; q++);
>> +
>> + name = av_strndup(p, q - p);
>> + if (!name) {
>> + err = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>> + if (hw_device_get_by_name(name)) {
>> + errmsg = "named device already exists";
>> + goto invalid;
>> + }
>> +
>> + p = q;
>> + } else {
>> + // Give the device an automatic name of the form "type%d".
>> + // We arbitrarily limit at 1000 anonymous devices of the same
>> + // type - there is probably something else very wrong if you
>> + // get to this limit.
>> + size_t index_pos;
>> + int index, index_limit = 1000;
>> + index_pos = strlen(type->name);
>> + name = av_malloc(index_pos + 4);
>> + if (!name) {
>> + err = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>> + for (index = 0; index < index_limit; index++) {
>> + snprintf(name, index_pos + 4, "%s%d", type->name, index);
>> + if (!hw_device_get_by_name(name))
>> + break;
>> + }
>> + if (index >= index_limit) {
>> + errmsg = "too many devices";
>> + goto invalid;
>> + }
>> + }
>> +
>> + if (!*p) {
>> + // New device with no parameters.
>> + err = av_hwdevice_ctx_create(&device_ref, type->type,
>> + NULL, NULL, 0);
>> + if (err < 0)
>> + goto fail;
>> +
>> + } else if (*p == ':') {
>> + // New device with some parameters.
>> + ++p;
>> + q = strchr(p, ',');
>> + if (q) {
>> + device = av_strndup(p, q - p);
>> + if (!device) {
>> + err = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>> + err = av_dict_parse_string(&options, q + 1, "=", ",", 0);
>> + if (err < 0) {
>> + errmsg = "failed to parse options";
>> + goto invalid;
>> + }
>> + }
>> +
>> + err = av_hwdevice_ctx_create(&device_ref, type->type,
>> + device ? device : p, options, 0);
>> + if (err < 0)
>> + goto fail;
>> +
>> + } else if (*p == '@') {
>> + // Derive from existing device.
>> +
>> + src = hw_device_get_by_name(p + 1);
>> + if (!src) {
>> + errmsg = "invalid source device name";
>> + goto invalid;
>> + }
>> +
>> + err = av_hwdevice_ctx_create_derived(&device_ref, type->type,
>> + src->device_ref, 0);
>> + if (err < 0)
>> + goto fail;
>> + } else {
>> + errmsg = "parse error";
>> + goto invalid;
>> + }
>> +
>> + dev = hw_device_new();
>> + if (!dev) {
>> + err = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>> +
>> + dev->name = name;
>> + dev->type = type->type;
>> + dev->device_ref = device_ref;
>> +
>> + if (dev_out)
>> + *dev_out = dev;
>> +
>> + name = NULL;
>> + err = 0;
>> +done:
>> + av_freep(&type_name);
>> + av_freep(&name);
>> + av_freep(&device);
>> + av_dict_free(&options);
>> + return err;
>> +invalid:
>> + av_log(NULL, AV_LOG_ERROR,
>> + "Invalid device specification \"%s\": %s\n", arg, errmsg);
>> + err = AVERROR(EINVAL);
>> + goto done;
>> +fail:
>> + av_log(NULL, AV_LOG_ERROR,
>> + "Device creation failed: %d.\n", err);
>> + goto done;
>
> Urgh, backwards gotos. Probably can be tolerated, but still a bit ugly.
>
>> +}
>> +
>> +void hw_device_free_all(void)
>> +{
>> + int i;
>> + for (i = 0; i < nb_hw_devices; i++) {
>> + av_freep(&hw_devices[i]->name);
>> + av_buffer_unref(&hw_devices[i]->device_ref);
>> + av_freep(&hw_devices[i]);
>> + }
>> + av_freep(&hw_devices);
>> + nb_hw_devices = 0;
>> +}
>> +
>> +static const HWDeviceType *hw_device_match_type_by_hwaccel(enum HWAccelID
>> hwaccel_id)
>> +{
>> + int i;
>> + if (hwaccel_id == HWACCEL_NONE)
>> + return NULL;
>> + for (i = 0; i < FF_ARRAY_ELEMS(hw_device_types); i++) {
>> + if (hw_device_types[i].hwaccel_id == hwaccel_id)
>> + return &hw_device_types[i];
>> + }
>> + return NULL;
>> +}
>
> (A bit annoying that the hwcontext API can't provide this, even though
> it has all information about it.)
There is some more rationalisation possible around the hwaccel setup if we
convert all devices in avconv; this function exists because we have two routes
into the setup here.
>> +
>> +static const HWDeviceType *hw_device_match_type_in_name(const char
>> *codec_name)
>> +{
>> + int i;
>> + for (i = 0; i < FF_ARRAY_ELEMS(hw_device_types); i++) {
>> + if (strstr(hw_device_types[i].name, codec_name))
>> + return &hw_device_types[i];
>> + }
>> + return NULL;
>> +}
>> +
>> +int hw_device_setup_for_decode(InputStream *ist)
>> +{
>> + const HWDeviceType *type;
>> + HWDevice *dev;
>> + int err;
>> +
>> + if (ist->hwaccel_device) {
>> + dev = hw_device_get_by_name(ist->hwaccel_device);
>> + if (!dev) {
>> + char *tmp;
>> + size_t len;
>> + type = hw_device_match_type_by_hwaccel(ist->hwaccel_id);
>> + if (!type) {
>> + // No match - this isn't necessarily invalid, though,
>> + // because an explicit device might not be needed or
>> + // the hwaccel setup could be handled elsewhere.
>> + return 0;
>> + }
>> + len = strlen(type->name) + 1 +
>> + strlen(ist->hwaccel_device) + 1;
>> + tmp = av_malloc(len);
>> + if (!tmp)
>> + return AVERROR(ENOMEM);
>> + snprintf(tmp, len, "%s:%s", type->name, ist->hwaccel_device);
>
> (Why does Libav libavutil have no asprintf wrapper?)
I think because Microsoft only implemented C99 snprintf in Windows 10 / MSVC
2015, and doing it without a proper snprintf basically requires writing all of
printf yourself.
>> + err = hw_device_init_from_string(tmp, &dev);
>> + av_free(tmp);
>> + if (err < 0)
>> + return err;
>> + }
>> + } else {
>> + if (ist->hwaccel_id != HWACCEL_NONE)
>> + type = hw_device_match_type_by_hwaccel(ist->hwaccel_id);
>> + else
>> + type = hw_device_match_type_in_name(ist->dec->name);
>> + if (type) {
>> + dev = hw_device_get_by_type(type->type);
>> + } else {
>> + // Hopefully we don't need a device.
>> + return 0;
>> + }
>
> (I'm a bit lost at what's even happening here, but maybe it's because
> of Monday...)
>
>> + }
>> +
>> + if (!dev)
>> + return AVERROR(ENODEV);
>> +
>> + ist->dec_ctx->hw_device_ctx = av_buffer_ref(dev->device_ref);
>> + if (!ist->dec_ctx->hw_device_ctx)
>> + return AVERROR(ENOMEM);
>> +
>> + return 0;
>> +}
>> +
>> +int hw_device_setup_for_encode(OutputStream *ost)
>> +{
>> + const HWDeviceType *type;
>> + HWDevice *dev;
>> +
>> + type = hw_device_match_type_in_name(ost->enc->name);
>> + if (type) {
>> + dev = hw_device_get_by_type(type->type);
>> + if (!dev)
>> + return AVERROR(ENODEV);
>> + ost->enc_ctx->hw_device_ctx = av_buffer_ref(dev->device_ref);
>> + if (!ost->enc_ctx->hw_device_ctx)
>> + return AVERROR(ENOMEM);
>> + return 0;
>> + } else {
>> + // Hopefully we don't need a device.
>> + return 0;
>> + }
>> +}
>> +
>> +static int hwaccel_retrieve_data(AVCodecContext *avctx, AVFrame *input)
>> +{
>> + InputStream *ist = avctx->opaque;
>> + AVFrame *output = 0;
>> + enum AVPixelFormat output_format = ist->hwaccel_output_format;
>> + int err;
>> +
>> + if (input->format == output_format) {
>> + // Nothing to do.
>> + return 0;
>> + }
>> +
>> + output = av_frame_alloc();
>> + if (!output)
>> + return AVERROR(ENOMEM);
>> +
>> + output->format = output_format;
>> +
>> + err = av_hwframe_transfer_data(output, input, 0);
>> + if (err < 0) {
>> + av_log(avctx, AV_LOG_ERROR, "Failed to transfer data to "
>> + "output frame: %d.\n", err);
>> + goto fail;
>> + }
>> +
>> + err = av_frame_copy_props(output, input);
>
> (Here I'm wondering why av_hwframe_transfer_data() isn't doing this
> automatically.)
It is explicitly transfer /data/. Maybe you were thinking of
av_hwframe_transfer_metadata()? :P
>> + if (err < 0) {
>> + av_frame_unref(output);
>> + goto fail;
>> + }
>> +
>> + av_frame_unref(input);
>> + av_frame_move_ref(input, output);
>> + av_frame_free(&output);
>> +
>> + return 0;
>> +
>> +fail:
>> + if (output)
>> + av_frame_free(&output);
>> + return err;
>> +}
>> +
>> +int hwaccel_decode_init(AVCodecContext *avctx)
>> +{
>> + InputStream *ist = avctx->opaque;
>> +
>> + ist->hwaccel_retrieve_data = &hwaccel_retrieve_data;
>> +
>> + return 0;
>> +}
>> diff --git a/avconv_opt.c b/avconv_opt.c
>> index e078a0b89..e6b7a9528 100644
>> --- a/avconv_opt.c
>> +++ b/avconv_opt.c
>> @@ -337,6 +337,11 @@ static int opt_vaapi_device(void *optctx, const char
>> *opt, const char *arg)
>> }
>> #endif
>>
>> +static int opt_init_hw_device(void *optctx, const char *opt, const char
>> *arg)
>> +{
>> + return hw_device_init_from_string(arg, NULL);
>> +}
>> +
>> /**
>> * Parse a metadata specifier passed as 'arg' parameter.
>> * @param arg metadata string to parse
>> @@ -2741,5 +2746,8 @@ const OptionDef options[] = {
>> "set VAAPI hardware device (DRM path or X11 display name)",
>> "device" },
>> #endif
>>
>> + { "init_hw_device", HAS_ARG | OPT_EXPERT, { .func_arg =
>> opt_init_hw_device },
>> + "initialise hardware device", "args" },
>> +
>> { NULL, },
>> };
>
> I'm a bit scared of the option syntax (and its parsing), but otherwise
> fine and only minor comments.
I think it is complex by necessity. If you have any ideas about how to
simplify the syntax and still achieve the same results then I am all ears.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
