Message ID | 39281f79-0625-8428-66b0-ef97acfc2c2f@jkqxz.net |
---|---|
State | New |
Headers | show |
On 2017/11/29 8:34, Mark Thompson wrote: > This removes all remaining device-type specificity. > --- > doc/examples/hw_decode.c | 55 ++++++++++++++++++++---------------------------- > 1 file changed, 23 insertions(+), 32 deletions(-) > > diff --git a/doc/examples/hw_decode.c b/doc/examples/hw_decode.c > index 9c7adbf51a..8f48f3b706 100644 > --- a/doc/examples/hw_decode.c > +++ b/doc/examples/hw_decode.c > @@ -44,34 +44,6 @@ static AVBufferRef *hw_device_ctx = NULL; > static enum AVPixelFormat hw_pix_fmt; > static FILE *output_file = NULL; > > -static enum AVPixelFormat find_fmt_by_hw_type(const enum AVHWDeviceType type) > -{ > - enum AVPixelFormat fmt; > - > - switch (type) { > - case AV_HWDEVICE_TYPE_VAAPI: > - fmt = AV_PIX_FMT_VAAPI; > - break; > - case AV_HWDEVICE_TYPE_DXVA2: > - fmt = AV_PIX_FMT_DXVA2_VLD; > - break; > - case AV_HWDEVICE_TYPE_D3D11VA: > - fmt = AV_PIX_FMT_D3D11; > - break; > - case AV_HWDEVICE_TYPE_VDPAU: > - fmt = AV_PIX_FMT_VDPAU; > - break; > - case AV_HWDEVICE_TYPE_VIDEOTOOLBOX: > - fmt = AV_PIX_FMT_VIDEOTOOLBOX; > - break; > - default: > - fmt = AV_PIX_FMT_NONE; > - break; > - } > - > - return fmt; > -} > - > static int hw_decoder_init(AVCodecContext *ctx, const enum AVHWDeviceType type) > { > int err = 0; > @@ -184,18 +156,23 @@ int main(int argc, char *argv[]) > AVCodec *decoder = NULL; > AVPacket packet; > enum AVHWDeviceType type; > + int i; > > if (argc < 4) { > - fprintf(stderr, "Usage: %s <vaapi|vdpau|dxva2|d3d11va> <input file> <output file>\n", argv[0]); > + fprintf(stderr, "Usage: %s <device type> <input file> <output file>\n", argv[0]); > return -1; > } > > av_register_all(); > > type = av_hwdevice_find_type_by_name(argv[1]); > - hw_pix_fmt = find_fmt_by_hw_type(type); > - if (hw_pix_fmt == -1) { > - fprintf(stderr, "Cannot support '%s' in this example.\n", argv[1]); > + if (type == AV_HWDEVICE_TYPE_NONE) { > + fprintf(stderr, "Device type %s is not supported.\n", argv[1]); > + fprintf(stderr, "Available device types:"); > + type = AV_HWDEVICE_TYPE_NONE; > + while((type = av_hwdevice_iterate_types(type)) != AV_HWDEVICE_TYPE_NONE) > + fprintf(stderr, " %s", av_hwdevice_get_type_name(type)); > + fprintf(stderr, "\n"); > return -1; > } > > @@ -218,6 +195,20 @@ int main(int argc, char *argv[]) > } > video_stream = ret; > > + for (i = 0;; i++) { > + const AVCodecHWConfig *config = avcodec_get_hw_config(decoder, i); > + if (!config) { > + fprintf(stderr, "Decoder %s does not support device type %s.\n", > + decoder->name, av_hwdevice_get_type_name(type)); > + return -1; > + } > + if (config->methods & AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX && > + config->device_type == type) { > + hw_pix_fmt = config->pix_fmt; > + break; > + } > + } > + > if (!(decoder_ctx = avcodec_alloc_context3(decoder))) > return AVERROR(ENOMEM); > LGTM and tesed
On Wed, Nov 29, 2017 at 00:34:03 +0000, Mark Thompson wrote: > + if (type == AV_HWDEVICE_TYPE_NONE) { > + fprintf(stderr, "Device type %s is not supported.\n", argv[1]); > + fprintf(stderr, "Available device types:"); > + type = AV_HWDEVICE_TYPE_NONE; > + while((type = av_hwdevice_iterate_types(type)) != AV_HWDEVICE_TYPE_NONE) > + fprintf(stderr, " %s", av_hwdevice_get_type_name(type)); I see what you're trying to do with "type = AV_HWDEVICE_TYPE_NONE" (initialize the iterator), but the assignment is redundant. I guess if the compiler optimizes it away anyway, it may be good for readability, especially since it's an example. *shrug* Moritz
Am 29.11.2017 um 13:22 schrieb Moritz Barsnick: > On Wed, Nov 29, 2017 at 00:34:03 +0000, Mark Thompson wrote: >> + if (type == AV_HWDEVICE_TYPE_NONE) { >> + fprintf(stderr, "Device type %s is not supported.\n", argv[1]); >> + fprintf(stderr, "Available device types:"); >> + type = AV_HWDEVICE_TYPE_NONE; >> + while((type = av_hwdevice_iterate_types(type)) != AV_HWDEVICE_TYPE_NONE) >> + fprintf(stderr, " %s", av_hwdevice_get_type_name(type)); > > I see what you're trying to do with "type = AV_HWDEVICE_TYPE_NONE" > (initialize the iterator), but the assignment is redundant. > > I guess if the compiler optimizes it away anyway, it may be good for > readability, especially since it's an example. *shrug* I fail to see how the assignment is redundant? Without the assignment, that would be an infinite loop.
2017-11-29 13:41 GMT+01:00 Timo Rothenpieler <timo@rothenpieler.org>: > Am 29.11.2017 um 13:22 schrieb Moritz Barsnick: >> >> On Wed, Nov 29, 2017 at 00:34:03 +0000, Mark Thompson wrote: >>> >>> + if (type == AV_HWDEVICE_TYPE_NONE) { >>> + fprintf(stderr, "Device type %s is not supported.\n", argv[1]); >>> + fprintf(stderr, "Available device types:"); >>> + type = AV_HWDEVICE_TYPE_NONE; >>> + while((type = av_hwdevice_iterate_types(type)) != >>> AV_HWDEVICE_TYPE_NONE) >>> + fprintf(stderr, " %s", av_hwdevice_get_type_name(type)); >> >> >> I see what you're trying to do with "type = AV_HWDEVICE_TYPE_NONE" >> (initialize the iterator), but the assignment is redundant. >> >> I guess if the compiler optimizes it away anyway, it may be good for >> readability, especially since it's an example. *shrug* > > > I fail to see how the assignment is redundant? > Without the assignment, that would be an infinite loop. One assignment above... Carl Eugen
On Wed, Nov 29, 2017 at 13:41:29 +0100, Timo Rothenpieler wrote: > Am 29.11.2017 um 13:22 schrieb Moritz Barsnick: > > On Wed, Nov 29, 2017 at 00:34:03 +0000, Mark Thompson wrote: > >> + if (type == AV_HWDEVICE_TYPE_NONE) { > >> + fprintf(stderr, "Device type %s is not supported.\n", argv[1]); > >> + fprintf(stderr, "Available device types:"); > >> + type = AV_HWDEVICE_TYPE_NONE; > >> + while((type = av_hwdevice_iterate_types(type)) != AV_HWDEVICE_TYPE_NONE) > >> + fprintf(stderr, " %s", av_hwdevice_get_type_name(type)); > > > > I see what you're trying to do with "type = AV_HWDEVICE_TYPE_NONE" > > (initialize the iterator), but the assignment is redundant. > [...] > I fail to see how the assignment is redundant? > Without the assignment, that would be an infinite loop. if (type == AV_HWDEVICE_TYPE_NONE) { // do something not affecting any variable type = AV_HWDEVICE_TYPE_NONE; Unless something else (what?) modifies the variable "type" in the meantime, its value was already guaranteed to be AV_HWDEVICE_TYPE_NONE. What am I missing? (I didn't mean "(type = av_hwdevice_iterate_types(type)) != AV_HWDEVICE_TYPE_NONE", that's why I explicitly quoted the assignment I meant.) Moritz
On 29/11/17 12:22, Moritz Barsnick wrote: > On Wed, Nov 29, 2017 at 00:34:03 +0000, Mark Thompson wrote: >> + if (type == AV_HWDEVICE_TYPE_NONE) { >> + fprintf(stderr, "Device type %s is not supported.\n", argv[1]); >> + fprintf(stderr, "Available device types:"); >> + type = AV_HWDEVICE_TYPE_NONE; >> + while((type = av_hwdevice_iterate_types(type)) != AV_HWDEVICE_TYPE_NONE) >> + fprintf(stderr, " %s", av_hwdevice_get_type_name(type)); > > I see what you're trying to do with "type = AV_HWDEVICE_TYPE_NONE" > (initialize the iterator), but the assignment is redundant. > > I guess if the compiler optimizes it away anyway, it may be good for > readability, especially since it's an example. *shrug* Huh, I didn't notice that. I've removed it - the iterating over devices is just to make the error message nicer and mostly irrelevant for the example, so I think avoiding the redundancy wins. Thanks, - Mark
On 29/11/17 00:48, Jun Zhao wrote: > On 2017/11/29 8:34, Mark Thompson wrote: >> This removes all remaining device-type specificity. >> --- >> doc/examples/hw_decode.c | 55 ++++++++++++++++++++---------------------------- >> 1 file changed, 23 insertions(+), 32 deletions(-) >> >> diff --git a/doc/examples/hw_decode.c b/doc/examples/hw_decode.c >> index 9c7adbf51a..8f48f3b706 100644 >> --- a/doc/examples/hw_decode.c >> +++ b/doc/examples/hw_decode.c >> @@ -44,34 +44,6 @@ static AVBufferRef *hw_device_ctx = NULL; >> static enum AVPixelFormat hw_pix_fmt; >> static FILE *output_file = NULL; >> >> -static enum AVPixelFormat find_fmt_by_hw_type(const enum AVHWDeviceType type) >> -{ >> - enum AVPixelFormat fmt; >> - >> - switch (type) { >> - case AV_HWDEVICE_TYPE_VAAPI: >> - fmt = AV_PIX_FMT_VAAPI; >> - break; >> - case AV_HWDEVICE_TYPE_DXVA2: >> - fmt = AV_PIX_FMT_DXVA2_VLD; >> - break; >> - case AV_HWDEVICE_TYPE_D3D11VA: >> - fmt = AV_PIX_FMT_D3D11; >> - break; >> - case AV_HWDEVICE_TYPE_VDPAU: >> - fmt = AV_PIX_FMT_VDPAU; >> - break; >> - case AV_HWDEVICE_TYPE_VIDEOTOOLBOX: >> - fmt = AV_PIX_FMT_VIDEOTOOLBOX; >> - break; >> - default: >> - fmt = AV_PIX_FMT_NONE; >> - break; >> - } >> - >> - return fmt; >> -} >> - >> static int hw_decoder_init(AVCodecContext *ctx, const enum AVHWDeviceType type) >> { >> int err = 0; >> @@ -184,18 +156,23 @@ int main(int argc, char *argv[]) >> AVCodec *decoder = NULL; >> AVPacket packet; >> enum AVHWDeviceType type; >> + int i; >> >> if (argc < 4) { >> - fprintf(stderr, "Usage: %s <vaapi|vdpau|dxva2|d3d11va> <input file> <output file>\n", argv[0]); >> + fprintf(stderr, "Usage: %s <device type> <input file> <output file>\n", argv[0]); >> return -1; >> } >> >> av_register_all(); >> >> type = av_hwdevice_find_type_by_name(argv[1]); >> - hw_pix_fmt = find_fmt_by_hw_type(type); >> - if (hw_pix_fmt == -1) { >> - fprintf(stderr, "Cannot support '%s' in this example.\n", argv[1]); >> + if (type == AV_HWDEVICE_TYPE_NONE) { >> + fprintf(stderr, "Device type %s is not supported.\n", argv[1]); >> + fprintf(stderr, "Available device types:"); >> + type = AV_HWDEVICE_TYPE_NONE; >> + while((type = av_hwdevice_iterate_types(type)) != AV_HWDEVICE_TYPE_NONE) >> + fprintf(stderr, " %s", av_hwdevice_get_type_name(type)); >> + fprintf(stderr, "\n"); >> return -1; >> } >> >> @@ -218,6 +195,20 @@ int main(int argc, char *argv[]) >> } >> video_stream = ret; >> >> + for (i = 0;; i++) { >> + const AVCodecHWConfig *config = avcodec_get_hw_config(decoder, i); >> + if (!config) { >> + fprintf(stderr, "Decoder %s does not support device type %s.\n", >> + decoder->name, av_hwdevice_get_type_name(type)); >> + return -1; >> + } >> + if (config->methods & AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX && >> + config->device_type == type) { >> + hw_pix_fmt = config->pix_fmt; >> + break; >> + } >> + } >> + >> if (!(decoder_ctx = avcodec_alloc_context3(decoder))) >> return AVERROR(ENOMEM); >> > LGTM and tesed Thanks! Applied with the fixup from Moritz. - Mark
diff --git a/doc/examples/hw_decode.c b/doc/examples/hw_decode.c index 9c7adbf51a..8f48f3b706 100644 --- a/doc/examples/hw_decode.c +++ b/doc/examples/hw_decode.c @@ -44,34 +44,6 @@ static AVBufferRef *hw_device_ctx = NULL; static enum AVPixelFormat hw_pix_fmt; static FILE *output_file = NULL; -static enum AVPixelFormat find_fmt_by_hw_type(const enum AVHWDeviceType type) -{ - enum AVPixelFormat fmt; - - switch (type) { - case AV_HWDEVICE_TYPE_VAAPI: - fmt = AV_PIX_FMT_VAAPI; - break; - case AV_HWDEVICE_TYPE_DXVA2: - fmt = AV_PIX_FMT_DXVA2_VLD; - break; - case AV_HWDEVICE_TYPE_D3D11VA: - fmt = AV_PIX_FMT_D3D11; - break; - case AV_HWDEVICE_TYPE_VDPAU: - fmt = AV_PIX_FMT_VDPAU; - break; - case AV_HWDEVICE_TYPE_VIDEOTOOLBOX: - fmt = AV_PIX_FMT_VIDEOTOOLBOX; - break; - default: - fmt = AV_PIX_FMT_NONE; - break; - } - - return fmt; -} - static int hw_decoder_init(AVCodecContext *ctx, const enum AVHWDeviceType type) { int err = 0; @@ -184,18 +156,23 @@ int main(int argc, char *argv[]) AVCodec *decoder = NULL; AVPacket packet; enum AVHWDeviceType type; + int i; if (argc < 4) { - fprintf(stderr, "Usage: %s <vaapi|vdpau|dxva2|d3d11va> <input file> <output file>\n", argv[0]); + fprintf(stderr, "Usage: %s <device type> <input file> <output file>\n", argv[0]); return -1; } av_register_all(); type = av_hwdevice_find_type_by_name(argv[1]); - hw_pix_fmt = find_fmt_by_hw_type(type); - if (hw_pix_fmt == -1) { - fprintf(stderr, "Cannot support '%s' in this example.\n", argv[1]); + if (type == AV_HWDEVICE_TYPE_NONE) { + fprintf(stderr, "Device type %s is not supported.\n", argv[1]); + fprintf(stderr, "Available device types:"); + type = AV_HWDEVICE_TYPE_NONE; + while((type = av_hwdevice_iterate_types(type)) != AV_HWDEVICE_TYPE_NONE) + fprintf(stderr, " %s", av_hwdevice_get_type_name(type)); + fprintf(stderr, "\n"); return -1; } @@ -218,6 +195,20 @@ int main(int argc, char *argv[]) } video_stream = ret; + for (i = 0;; i++) { + const AVCodecHWConfig *config = avcodec_get_hw_config(decoder, i); + if (!config) { + fprintf(stderr, "Decoder %s does not support device type %s.\n", + decoder->name, av_hwdevice_get_type_name(type)); + return -1; + } + if (config->methods & AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX && + config->device_type == type) { + hw_pix_fmt = config->pix_fmt; + break; + } + } + if (!(decoder_ctx = avcodec_alloc_context3(decoder))) return AVERROR(ENOMEM);