diff mbox series

[FFmpeg-devel,v3,2/2] ffmpeg_opt: consider HW acceleration method when selecting decoder

Message ID 20220725043051.9692-2-haihao.xiang@intel.com
State New
Headers show
Series [FFmpeg-devel,v3,1/2] ffmpeg_opt: select a decoder after getting values for per-stream hwdec options | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Xiang, Haihao July 25, 2022, 4:30 a.m. UTC
From: Haihao Xiang <haihao.xiang@intel.com>

Usually a HW decoder is expected when user specifies a HW acceleration
method via -hwaccel option, however the current implementation doesn't
take HW acceleration method into account, it is possible to select a SW
decoder.

For example:
$ ffmpeg -hwaccel vaapi -i av1.mp4 -f null -
$ ffmpeg -hwaccel nvdec -i av1.mp4 -f null -
$ ffmpeg -hwaccel vdpau -i av1.mp4 -f null -
[...]
Stream #0:0 -> #0:0 (av1 (libdav1d) -> wrapped_avframe (native))

libdav1d is selected in this case even if vaapi, nvdec or vdpau is
specified.

After applying this patch, the native av1 decoder (with vaapi, nvdec or
vdpau support) is selected for decoding(libdav1d is still used for
probing format).
$ ffmpeg -hwaccel vaapi -i av1.mp4 -f null -
$ ffmpeg -hwaccel nvdec -i av1.mp4 -f null -
$ ffmpeg -hwaccel vdpau -i av1.mp4 -f null -
[...]
Stream #0:0 -> #0:0 (av1 (native) -> wrapped_avframe (native))

Tested-by: Mario Roy <marioeroy@gmail.com>
Signed-off-by: Haihao Xiang <haihao.xiang@intel.com>
---
 fftools/ffmpeg_opt.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

Comments

Xiang, Haihao July 29, 2022, 8:37 a.m. UTC | #1
On Mon, 2022-07-25 at 12:30 +0800, Xiang, Haihao wrote:
> From: Haihao Xiang <haihao.xiang@intel.com>
> 
> Usually a HW decoder is expected when user specifies a HW acceleration
> method via -hwaccel option, however the current implementation doesn't
> take HW acceleration method into account, it is possible to select a SW
> decoder.
> 
> For example:
> $ ffmpeg -hwaccel vaapi -i av1.mp4 -f null -
> $ ffmpeg -hwaccel nvdec -i av1.mp4 -f null -
> $ ffmpeg -hwaccel vdpau -i av1.mp4 -f null -
> [...]
> Stream #0:0 -> #0:0 (av1 (libdav1d) -> wrapped_avframe (native))
> 
> libdav1d is selected in this case even if vaapi, nvdec or vdpau is
> specified.
> 
> After applying this patch, the native av1 decoder (with vaapi, nvdec or
> vdpau support) is selected for decoding(libdav1d is still used for
> probing format).
> $ ffmpeg -hwaccel vaapi -i av1.mp4 -f null -
> $ ffmpeg -hwaccel nvdec -i av1.mp4 -f null -
> $ ffmpeg -hwaccel vdpau -i av1.mp4 -f null -
> [...]
> Stream #0:0 -> #0:0 (av1 (native) -> wrapped_avframe (native))
> 
> Tested-by: Mario Roy <marioeroy@gmail.com>
> Signed-off-by: Haihao Xiang <haihao.xiang@intel.com>
> ---
>  fftools/ffmpeg_opt.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 2771c5d993..614bfeea80 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -861,6 +861,48 @@ static const AVCodec *choose_decoder(OptionsContext *o,
> AVFormatContext *s, AVSt
>          return avcodec_find_decoder(st->codecpar->codec_id);
>  }
>  
> +static const AVCodec *choose_decoder2(InputStream *ist, OptionsContext *o,
> AVFormatContext *s, AVStream *st)
> +{
> +    char *codec_name = NULL;
> +
> +    MATCH_PER_STREAM_OPT(codec_names, str, codec_name, s, st);
> +    if (codec_name) {
> +        const AVCodec *codec = find_codec_or_die(codec_name, st->codecpar-
> >codec_type, 0);
> +        st->codecpar->codec_id = codec->id;
> +        if (recast_media && st->codecpar->codec_type != codec->type)
> +            st->codecpar->codec_type = codec->type;
> +        return codec;
> +    } else {
> +        if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
> +            ist->hwaccel_id == HWACCEL_GENERIC &&
> +            ist->hwaccel_device_type != AV_HWDEVICE_TYPE_NONE) {
> +            const AVCodec *p;
> +            void *i = 0;
> +
> +            while ((p = av_codec_iterate(&i))) {
> +                int j;
> +
> +                if (p->id != st->codecpar->codec_id ||
> +                    !av_codec_is_decoder(p) ||
> +                    !avcodec_get_hw_config(p, 0))
> +                    continue;
> +
> +                for (j = 0; ;j++) {
> +                    const AVCodecHWConfig *config = avcodec_get_hw_config(p,
> j);
> +
> +                    if (!config)
> +                        break;
> +
> +                    if (config->device_type == ist->hwaccel_device_type)
> +                        return p;
> +                }
> +            }
> +        }
> +
> +        return avcodec_find_decoder(st->codecpar->codec_id);
> +    }
> +}
> +
>  /* Add all the streams from the given input file to the global
>   * list of input streams. */
>  static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
> @@ -973,7 +1015,7 @@ static void add_input_streams(OptionsContext *o,
> AVFormatContext *ic)
>              ist->hwaccel_pix_fmt = AV_PIX_FMT_NONE;
>          }
>  
> -        ist->dec = choose_decoder(o, ic, st);
> +        ist->dec = choose_decoder2(ist, o, ic, st);
>          ist->decoder_opts = filter_codec_opts(o->g->codec_opts, ist->st-
> >codecpar->codec_id, ic, st, ist->dec);
>  
>          ist->reinit_filters = -1;

Hi,

libdav1d is the preferred AV1 decoder in FFmpeg, libdav1d is always used when
running the command below even if user expects vaapi or other HW acceleration
methods.

$ ffmpeg -hwaccel vaapi -i av1.ivf -f null -

A similar command works as expected for other codecs, e.g. GPU is used when
running the command below (see 
http://ffmpeg.org/pipermail/ffmpeg-user/2022-July/055227.html)

$ ffmpeg -y -hwaccel vdpau -i input_vp9.webm output.ts

This patchset can ensure FFmpeg chooses a HW accelerated decoder when '-hwaccel
arg' is specified in the command line. 

Is there any comment for this patchset ?

Thanks
Haihao
Ronald S. Bultje July 29, 2022, 8:50 a.m. UTC | #2
Hi,

On Fri, Jul 29, 2022 at 4:38 PM Xiang, Haihao <
haihao.xiang-at-intel.com@ffmpeg.org> wrote:

> libdav1d is the preferred AV1 decoder in FFmpeg, libdav1d is always used
> when
> running the command below even if user expects vaapi or other HW
> acceleration
> methods.
>

I think that is a pretty serious issue. Don't we always want to prefer a hw
decoder by default? I agree there should also be ways to force-select
specific hw/sw decoders, but shouldn't hw be the default?

Ronald
Soft Works July 29, 2022, 9:46 a.m. UTC | #3
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Ronald S. Bultje
> Sent: Friday, July 29, 2022 10:50 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v3 2/2] ffmpeg_opt: consider HW
> acceleration method when selecting decoder
> 
> Hi,
> 
> On Fri, Jul 29, 2022 at 4:38 PM Xiang, Haihao <
> haihao.xiang-at-intel.com@ffmpeg.org> wrote:
> 
> > libdav1d is the preferred AV1 decoder in FFmpeg, libdav1d is always
> used
> > when
> > running the command below even if user expects vaapi or other HW
> > acceleration
> > methods.
> >
> 
> I think that is a pretty serious issue. Don't we always want to
> prefer a hw
> decoder by default? I agree there should also be ways to force-select
> specific hw/sw decoders, but shouldn't hw be the default?

I don't think that this would be reasonably possible in any way.
There are a lot of questions which ffmpeg cannot answer, e.g.:

- Which hwaccel to choose?
- Which hwaccel is available?
- Which parameters are required for selecting a device 
  that is working?
- Is the auto-selected device even capable to decode a certain
  input?
  (pixel format, bit depth, codec profile, frame size, ...)

For the user who is creating the command line, it is important to
be able to rely on what is going to happen. If they can't command
lines will fail:

- The outputs of hw decoders vary. Some output to hw format, some
  to sw format by default. You may need to specify hwaccel_output_format
  or use hwdownload
- Depending on the hwaccel, a totally different set of filters
  may be required (not to speak of encoders)
- How would you specify that you want a sw decoder instead?

IMO, predictability is the topmost important behavioral aspect
for users (be it humans of machines.. ;-)

Best regards,
softworkz
Xiang, Haihao Aug. 1, 2022, 1:52 a.m. UTC | #4
On Fri, 2022-07-29 at 09:46 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Ronald S. Bultje
> > Sent: Friday, July 29, 2022 10:50 AM
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH v3 2/2] ffmpeg_opt: consider HW
> > acceleration method when selecting decoder
> > 
> > Hi,
> > 
> > On Fri, Jul 29, 2022 at 4:38 PM Xiang, Haihao <
> > haihao.xiang-at-intel.com@ffmpeg.org> wrote:
> > 
> > > libdav1d is the preferred AV1 decoder in FFmpeg, libdav1d is always
> > 
> > used
> > > when
> > > running the command below even if user expects vaapi or other HW
> > > acceleration
> > > methods.
> > > 
> > 
> > I think that is a pretty serious issue. Don't we always want to
> > prefer a hw
> > decoder by default? I agree there should also be ways to force-select
> > specific hw/sw decoders, but shouldn't hw be the default?
> 
> I don't think that this would be reasonably possible in any way.
> There are a lot of questions which ffmpeg cannot answer, e.g.:
> 
> - Which hwaccel to choose?
> - Which hwaccel is available?
> - Which parameters are required for selecting a device 
>   that is working?
> - Is the auto-selected device even capable to decode a certain
>   input?
>   (pixel format, bit depth, codec profile, frame size, ...)
> 
> For the user who is creating the command line, it is important to
> be able to rely on what is going to happen. If they can't command
> lines will fail:
> 
> - The outputs of hw decoders vary. Some output to hw format, some
>   to sw format by default. You may need to specify hwaccel_output_format
>   or use hwdownload
> - Depending on the hwaccel, a totally different set of filters
>   may be required (not to speak of encoders)
> - How would you specify that you want a sw decoder instead?
> 
> IMO, predictability is the topmost important behavioral aspect
> for users (be it humans of machines.. ;-)


I agree with you it would be better not to make hw the default if user doesn't
ask for. However '-hwaccel arg' is used to specify hw acceleration method in the
examples. According to doc (
https://github.com/FFmpeg/FFmpeg/blob/master/doc/ffmpeg.texi#L1260-L1262), a hwdecoder is expected. 

"@item -hwaccel[:@var{stream_specifier}] @var{hwaccel} (@emph{input,per-stream})
Use hardware acceleration to decode the matching stream(s). "

This patch is to fix the issue that a SW decoder is chosen even if user is
expecting a hw method, not to make hw the default in any case.

Thanks
Haihao
Soft Works Aug. 1, 2022, 2:14 a.m. UTC | #5
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Xiang, Haihao
> Sent: Monday, August 1, 2022 3:53 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v3 2/2] ffmpeg_opt: consider HW
> acceleration method when selecting decoder
> 
> On Fri, 2022-07-29 at 09:46 +0000, Soft Works wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Ronald S. Bultje
> > > Sent: Friday, July 29, 2022 10:50 AM
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH v3 2/2] ffmpeg_opt: consider
> HW
> > > acceleration method when selecting decoder
> > >
> > > Hi,
> > >
> > > On Fri, Jul 29, 2022 at 4:38 PM Xiang, Haihao <
> > > haihao.xiang-at-intel.com@ffmpeg.org> wrote:
> > >
> > > > libdav1d is the preferred AV1 decoder in FFmpeg, libdav1d is
> always
> > >
> > > used
> > > > when
> > > > running the command below even if user expects vaapi or other
> HW
> > > > acceleration
> > > > methods.
> > > >
> > >
> > > I think that is a pretty serious issue. Don't we always want to
> > > prefer a hw
> > > decoder by default? I agree there should also be ways to force-
> select
> > > specific hw/sw decoders, but shouldn't hw be the default?
> >
> > I don't think that this would be reasonably possible in any way.
> > There are a lot of questions which ffmpeg cannot answer, e.g.:
> >
> > - Which hwaccel to choose?
> > - Which hwaccel is available?
> > - Which parameters are required for selecting a device
> >   that is working?
> > - Is the auto-selected device even capable to decode a certain
> >   input?
> >   (pixel format, bit depth, codec profile, frame size, ...)
> >
> > For the user who is creating the command line, it is important to
> > be able to rely on what is going to happen. If they can't command
> > lines will fail:
> >
> > - The outputs of hw decoders vary. Some output to hw format, some
> >   to sw format by default. You may need to specify
> hwaccel_output_format
> >   or use hwdownload
> > - Depending on the hwaccel, a totally different set of filters
> >   may be required (not to speak of encoders)
> > - How would you specify that you want a sw decoder instead?
> >
> > IMO, predictability is the topmost important behavioral aspect
> > for users (be it humans of machines.. ;-)
> 
> 
> I agree with you it would be better not to make hw the default if
> user doesn't
> ask for. However '-hwaccel arg' is used to specify hw acceleration
> method in the
> examples. According to doc (
> https://github.com/FFmpeg/FFmpeg/blob/master/doc/ffmpeg.texi#L1260-
> L1262), a hwdecoder is expected.
> 
> "@item -hwaccel[:@var{stream_specifier}] @var{hwaccel}
> (@emph{input,per-stream})
> Use hardware acceleration to decode the matching stream(s). "
> 
> This patch is to fix the issue that a SW decoder is chosen even if
> user is
> expecting a hw method, not to make hw the default in any case.
> 
> Thanks
> Haihao
> 

Oh, I'm in no way opposed to your patch, I just talked about the
idea of a general automatic selection of hardware decoders.

I agree that the behavior for AV1 should be the same as for the other
decoders.

Best wishes,
softworkz
Xiang, Haihao Aug. 1, 2022, 2:37 a.m. UTC | #6
On Mon, 2022-08-01 at 02:14 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Xiang, Haihao
> > Sent: Monday, August 1, 2022 3:53 AM
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v3 2/2] ffmpeg_opt: consider HW
> > acceleration method when selecting decoder
> > 
> > On Fri, 2022-07-29 at 09:46 +0000, Soft Works wrote:
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > > Ronald S. Bultje
> > > > Sent: Friday, July 29, 2022 10:50 AM
> > > > To: FFmpeg development discussions and patches <ffmpeg-
> > > > devel@ffmpeg.org>
> > > > Subject: Re: [FFmpeg-devel] [PATCH v3 2/2] ffmpeg_opt: consider
> > 
> > HW
> > > > acceleration method when selecting decoder
> > > > 
> > > > Hi,
> > > > 
> > > > On Fri, Jul 29, 2022 at 4:38 PM Xiang, Haihao <
> > > > haihao.xiang-at-intel.com@ffmpeg.org> wrote:
> > > > 
> > > > > libdav1d is the preferred AV1 decoder in FFmpeg, libdav1d is
> > 
> > always
> > > > 
> > > > used
> > > > > when
> > > > > running the command below even if user expects vaapi or other
> > 
> > HW
> > > > > acceleration
> > > > > methods.
> > > > > 
> > > > 
> > > > I think that is a pretty serious issue. Don't we always want to
> > > > prefer a hw
> > > > decoder by default? I agree there should also be ways to force-
> > 
> > select
> > > > specific hw/sw decoders, but shouldn't hw be the default?
> > > 
> > > I don't think that this would be reasonably possible in any way.
> > > There are a lot of questions which ffmpeg cannot answer, e.g.:
> > > 
> > > - Which hwaccel to choose?
> > > - Which hwaccel is available?
> > > - Which parameters are required for selecting a device
> > >   that is working?
> > > - Is the auto-selected device even capable to decode a certain
> > >   input?
> > >   (pixel format, bit depth, codec profile, frame size, ...)
> > > 
> > > For the user who is creating the command line, it is important to
> > > be able to rely on what is going to happen. If they can't command
> > > lines will fail:
> > > 
> > > - The outputs of hw decoders vary. Some output to hw format, some
> > >   to sw format by default. You may need to specify
> > 
> > hwaccel_output_format
> > >   or use hwdownload
> > > - Depending on the hwaccel, a totally different set of filters
> > >   may be required (not to speak of encoders)
> > > - How would you specify that you want a sw decoder instead?
> > > 
> > > IMO, predictability is the topmost important behavioral aspect
> > > for users (be it humans of machines.. ;-)
> > 
> > 
> > I agree with you it would be better not to make hw the default if
> > user doesn't
> > ask for. However '-hwaccel arg' is used to specify hw acceleration
> > method in the
> > examples. According to doc (
> > https://github.com/FFmpeg/FFmpeg/blob/master/doc/ffmpeg.texi#L1260-
> > L1262), a hwdecoder is expected.
> > 
> > "@item -hwaccel[:@var{stream_specifier}] @var{hwaccel}
> > (@emph{input,per-stream})
> > Use hardware acceleration to decode the matching stream(s). "
> > 
> > This patch is to fix the issue that a SW decoder is chosen even if
> > user is
> > expecting a hw method, not to make hw the default in any case.
> > 
> > Thanks
> > Haihao
> > 
> 
> Oh, I'm in no way opposed to your patch, I just talked about the
> idea of a general automatic selection of hardware decoders.
> 

Sorry for misreading your email and thanks for your clarifying. 

> I agree that the behavior for AV1 should be the same as for the other
> decoders.

Thanks, I will push it if no more comment in a few days.

- Haihao
Anton Khirnov Aug. 1, 2022, 9:12 a.m. UTC | #7
Hi,
the concept is generally ok, but I have a few comments on the
implementation.

Quoting Xiang, Haihao (2022-07-25 06:30:51)
> +static const AVCodec *choose_decoder2(InputStream *ist, OptionsContext *o, AVFormatContext *s, AVStream *st)
> +{
> +    char *codec_name = NULL;
> +
> +    MATCH_PER_STREAM_OPT(codec_names, str, codec_name, s, st);
> +    if (codec_name) {
> +        const AVCodec *codec = find_codec_or_die(codec_name, st->codecpar->codec_type, 0);
> +        st->codecpar->codec_id = codec->id;
> +        if (recast_media && st->codecpar->codec_type != codec->type)
> +            st->codecpar->codec_type = codec->type;
> +        return codec;
> +    } else {
> +        if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
> +            ist->hwaccel_id == HWACCEL_GENERIC &&
> +            ist->hwaccel_device_type != AV_HWDEVICE_TYPE_NONE) {
> +            const AVCodec *p;

'p' is a rather weird name for an AVCodec instance, I'd expect 'c' or
'codec' or something like that

> +            void *i = 0;

NULL, it's a pointer

> +
> +            while ((p = av_codec_iterate(&i))) {
> +                int j;
> +
> +                if (p->id != st->codecpar->codec_id ||
> +                    !av_codec_is_decoder(p) ||
> +                    !avcodec_get_hw_config(p, 0))
> +                    continue;
> +
> +                for (j = 0; ;j++) {

for (int j = 0; config = avcodec_get_hw_config(p, j); j++)

gets rid of the explicit check in the loop and also
avcodec_get_hw_config() in the condition above

> +                    const AVCodecHWConfig *config = avcodec_get_hw_config(p, j);
> +
> +                    if (!config)
> +                        break;
> +
> +                    if (config->device_type == ist->hwaccel_device_type)
> +                        return p;
> +                }
> +            }
> +        }
> +
> +        return avcodec_find_decoder(st->codecpar->codec_id);
> +    }
> +}
> +
>  /* Add all the streams from the given input file to the global
>   * list of input streams. */
>  static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
> @@ -973,7 +1015,7 @@ static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
>              ist->hwaccel_pix_fmt = AV_PIX_FMT_NONE;
>          }
>  
> -        ist->dec = choose_decoder(o, ic, st);
> +        ist->dec = choose_decoder2(ist, o, ic, st);

I don't like adding a new function that partially duplicates
choose_decoder() and has a non-descriptive name. Just extend
choose_decoder() by passing hwaccel_id and hwaccel_device_type to it.
Xiang, Haihao Aug. 2, 2022, 7:57 a.m. UTC | #8
On Mon, 2022-08-01 at 11:12 +0200, Anton Khirnov wrote:
> Hi,
> the concept is generally ok, but I have a few comments on the
> implementation.
> 
> Quoting Xiang, Haihao (2022-07-25 06:30:51)
> > +static const AVCodec *choose_decoder2(InputStream *ist, OptionsContext *o,
> > AVFormatContext *s, AVStream *st)
> > +{
> > +    char *codec_name = NULL;
> > +
> > +    MATCH_PER_STREAM_OPT(codec_names, str, codec_name, s, st);
> > +    if (codec_name) {
> > +        const AVCodec *codec = find_codec_or_die(codec_name, st->codecpar-
> > >codec_type, 0);
> > +        st->codecpar->codec_id = codec->id;
> > +        if (recast_media && st->codecpar->codec_type != codec->type)
> > +            st->codecpar->codec_type = codec->type;
> > +        return codec;
> > +    } else {
> > +        if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
> > +            ist->hwaccel_id == HWACCEL_GENERIC &&
> > +            ist->hwaccel_device_type != AV_HWDEVICE_TYPE_NONE) {
> > +            const AVCodec *p;
> 
> 'p' is a rather weird name for an AVCodec instance, I'd expect 'c' or
> 'codec' or something like that
> 
> > +            void *i = 0;
> 
> NULL, it's a pointer
> 
> > +
> > +            while ((p = av_codec_iterate(&i))) {
> > +                int j;
> > +
> > +                if (p->id != st->codecpar->codec_id ||
> > +                    !av_codec_is_decoder(p) ||
> > +                    !avcodec_get_hw_config(p, 0))
> > +                    continue;
> > +
> > +                for (j = 0; ;j++) {
> 
> for (int j = 0; config = avcodec_get_hw_config(p, j); j++)
> 
> gets rid of the explicit check in the loop and also
> avcodec_get_hw_config() in the condition above
> 
> > +                    const AVCodecHWConfig *config =
> > avcodec_get_hw_config(p, j);
> > +
> > +                    if (!config)
> > +                        break;
> > +
> > +                    if (config->device_type == ist->hwaccel_device_type)
> > +                        return p;
> > +                }
> > +            }
> > +        }
> > +
> > +        return avcodec_find_decoder(st->codecpar->codec_id);
> > +    }
> > +}
> > +
> >  /* Add all the streams from the given input file to the global
> >   * list of input streams. */
> >  static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
> > @@ -973,7 +1015,7 @@ static void add_input_streams(OptionsContext *o,
> > AVFormatContext *ic)
> >              ist->hwaccel_pix_fmt = AV_PIX_FMT_NONE;
> >          }
> >  
> > -        ist->dec = choose_decoder(o, ic, st);
> > +        ist->dec = choose_decoder2(ist, o, ic, st);
> 
> I don't like adding a new function that partially duplicates
> choose_decoder() and has a non-descriptive name. Just extend
> choose_decoder() by passing hwaccel_id and hwaccel_device_type to it.

Thanks for the comments, fixed in v4

-Haihao
diff mbox series

Patch

diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 2771c5d993..614bfeea80 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -861,6 +861,48 @@  static const AVCodec *choose_decoder(OptionsContext *o, AVFormatContext *s, AVSt
         return avcodec_find_decoder(st->codecpar->codec_id);
 }
 
+static const AVCodec *choose_decoder2(InputStream *ist, OptionsContext *o, AVFormatContext *s, AVStream *st)
+{
+    char *codec_name = NULL;
+
+    MATCH_PER_STREAM_OPT(codec_names, str, codec_name, s, st);
+    if (codec_name) {
+        const AVCodec *codec = find_codec_or_die(codec_name, st->codecpar->codec_type, 0);
+        st->codecpar->codec_id = codec->id;
+        if (recast_media && st->codecpar->codec_type != codec->type)
+            st->codecpar->codec_type = codec->type;
+        return codec;
+    } else {
+        if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
+            ist->hwaccel_id == HWACCEL_GENERIC &&
+            ist->hwaccel_device_type != AV_HWDEVICE_TYPE_NONE) {
+            const AVCodec *p;
+            void *i = 0;
+
+            while ((p = av_codec_iterate(&i))) {
+                int j;
+
+                if (p->id != st->codecpar->codec_id ||
+                    !av_codec_is_decoder(p) ||
+                    !avcodec_get_hw_config(p, 0))
+                    continue;
+
+                for (j = 0; ;j++) {
+                    const AVCodecHWConfig *config = avcodec_get_hw_config(p, j);
+
+                    if (!config)
+                        break;
+
+                    if (config->device_type == ist->hwaccel_device_type)
+                        return p;
+                }
+            }
+        }
+
+        return avcodec_find_decoder(st->codecpar->codec_id);
+    }
+}
+
 /* Add all the streams from the given input file to the global
  * list of input streams. */
 static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
@@ -973,7 +1015,7 @@  static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
             ist->hwaccel_pix_fmt = AV_PIX_FMT_NONE;
         }
 
-        ist->dec = choose_decoder(o, ic, st);
+        ist->dec = choose_decoder2(ist, o, ic, st);
         ist->decoder_opts = filter_codec_opts(o->g->codec_opts, ist->st->codecpar->codec_id, ic, st, ist->dec);
 
         ist->reinit_filters = -1;