[FFmpeg-devel] [PATCH v2 1/3] avutil/hwcontext: add a function to get the AVHWDeviceType

[Wu, Tong1]

> > On Jun 30, 2022, at 7:56 PM, Andreas Rheinhardt
> <andreas.rheinhardt at outlook.com> wrote:
> >
> > "zhilizhao(赵志立)":
> >>
> >>
> >>> On Jun 30, 2022, at 4:43 PM, Anton Khirnov <anton at khirnov.net> wrote:
> >>>
> >>> Quoting Tong Wu (2022-06-30 04:45:56)
> >>>> Add a function to get the corresponding AVHWDeviceType from a given
> >>>> hardware pixel format.
> >>>>
> >>>> Signed-off-by: Tong Wu <tong1.wu at intel.com>
> >>>> ---
> >>>> libavutil/hwcontext.c | 12 ++++++++++++ libavutil/hwcontext.h |  9
> >>>> +++++++++
> >>>> 2 files changed, 21 insertions(+)
> >>>>
> >>>> diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c index
> >>>> ab9ad3703e..3521ed34f4 100644
> >>>> --- a/libavutil/hwcontext.c
> >>>> +++ b/libavutil/hwcontext.c
> >>>> @@ -80,6 +80,18 @@ static const char *const hw_type_names[] = {
> >>>>    [AV_HWDEVICE_TYPE_VULKAN] = "vulkan", };
> >>>>
> >>>> +enum AVHWDeviceType av_hwdevice_get_type_by_pix_fmt(enum
> >>>> +AVPixelFormat fmt) {
> >>>> +    int i, j;
> >>>
> >>> Nit: you can and should declare loop indices in the loop statement
> >>> itself

Sure, will modify them in v3.

> >>>
> >>>> +    for (i = 0; hw_table[i]; i++) {
> >>>> +        for (j = 0; hw_table[i]->pix_fmts[j] != AV_PIX_FMT_NONE; j++) {
> >>>> +            if (hw_table[i]->pix_fmts[j] == fmt)
> >>>> +                return hw_table[i]->type;
> >>>> +        }
> >>>> +    }
> >>>> +    return AV_HWDEVICE_TYPE_NONE;
> >>>> +}
> >>>> +
> >>>> enum AVHWDeviceType av_hwdevice_find_type_by_name(const char
> *name)
> >>>> {
> >>>>    int type;
> >>>> diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h index
> >>>> c18b7e1e8b..97f94403e2 100644
> >>>> --- a/libavutil/hwcontext.h
> >>>> +++ b/libavutil/hwcontext.h
> >>>> @@ -229,6 +229,15 @@ typedef struct AVHWFramesContext {
> >>>>    int width, height;
> >>>> } AVHWFramesContext;
> >>>>
> >>>> +/**
> >>>> + * Get the device type by a given pixel format.
> >>>> + *
> >>>> + * @param fmt Pixel format from enum AVPixelFormat.
> >>>> + * @return The type from enum AVHWDeviceType, or
> AV_HWDEVICE_TYPE_NONE if
> >>>> + *         not found.
> >>>> + */
> >>>> +enum AVHWDeviceType av_hwdevice_get_type_by_pix_fmt(enum
> >>>> +AVPixelFormat fmt);
> >>>
> >>> I wonder if we should consider the possibility of a format being
> >>> supported by more than one device type.
> >>
> >> For future proof, we can make it clear that there is no guarantee
> >> that the device type is unique, e.g.,
> >>
> >> "Get any device type which support the given pixel format”
> >>
> >
> > Then you'd need to return a list or modify the user accept an iterator.
> 
> Iterator should be fine.
> 
> However, the use case is unclear: since we only return an AVHWDeviceType
> without description like av_pix_fmt_desc_get(), user has little information to
> skip to the next one, unless user only wants to get all of the device types
> which support a pixel format.

Since so far there's no hardware format being supported by more than one hardware device, can we just keep current implementation and clarify it in comments such as "Get any device type which support the given pixel format”? 

Thanks,
Tong

> 
> >
> > - Andreas
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request at ffmpeg.org
> with subject "unsubscribe".