Message ID | 20220630024558.1444-1-tong1.wu@intel.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,v2,1/3] avutil/hwcontext: add a function to get the AVHWDeviceType | expand |
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 |
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@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 > + 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.
> On Jun 30, 2022, at 4:43 PM, Anton Khirnov <anton@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@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 > >> + 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” > > -- > Anton Khirnov > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
"zhilizhao(赵志立)": > > >> On Jun 30, 2022, at 4:43 PM, Anton Khirnov <anton@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@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 >> >>> + 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. - Andreas
> On Jun 30, 2022, at 7:56 PM, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > "zhilizhao(赵志立)": >> >> >>> On Jun 30, 2022, at 4:43 PM, Anton Khirnov <anton@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@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 >>> >>>> + 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. > > - Andreas > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> > On Jun 30, 2022, at 7:56 PM, Andreas Rheinhardt > <andreas.rheinhardt@outlook.com> wrote: > > > > "zhilizhao(赵志立)": > >> > >> > >>> On Jun 30, 2022, at 4:43 PM, Anton Khirnov <anton@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@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@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org > with subject "unsubscribe".
> > > On Jun 30, 2022, at 7:56 PM, Andreas Rheinhardt > > <andreas.rheinhardt@outlook.com> wrote: > > > > > > "zhilizhao(赵志立)": > > >> > > >> > > >>> On Jun 30, 2022, at 4:43 PM, Anton Khirnov <anton@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@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 > Plus, do you think adding a AV_PIX_FMT_FLAG_HWACCEL check for the input pixel format and change the function name to av_hwdevice_get_type_by_hwaccel_pix_fmt will help? Let users know only hardware pixel formats are acceptable for this function and like I said at this moment not any hardware format is supported by more than one hardware devices. Tong > > > > > > > > - Andreas > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org > with subject "unsubscribe".
Quoting Wu, Tong1 (2022-07-01 08:51:28) > Plus, do you think adding a AV_PIX_FMT_FLAG_HWACCEL check for the > input pixel format and change the function name to > av_hwdevice_get_type_by_hwaccel_pix_fmt will help? I'd prefer not to, that name is way too long > Let users know only hardware pixel formats are acceptable for this > function and like I said at this moment not any hardware format is > supported by more than one hardware devices. Maybe just document in the doxy that it will return the preferred device type for a pixel format. Overall I don't insist on any changes here, mainly wondering if anyone can think of a potential future case where we'd want a single pixel format supported by multiple devices
> Quoting Wu, Tong1 (2022-07-01 08:51:28) > > Plus, do you think adding a AV_PIX_FMT_FLAG_HWACCEL check for the > > input pixel format and change the function name to > > av_hwdevice_get_type_by_hwaccel_pix_fmt will help? > > I'd prefer not to, that name is way too long > > > Let users know only hardware pixel formats are acceptable for this > > function and like I said at this moment not any hardware format is > > supported by more than one hardware devices. > > Maybe just document in the doxy that it will return the preferred device type > for a pixel format. Sure, will do it in v3. Thanks for the comments. > > Overall I don't insist on any changes here, mainly wondering if anyone can > think of a potential future case where we'd want a single pixel format > supported by multiple devices > > -- > Anton Khirnov > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org > with subject "unsubscribe".
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; + 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); + /** * Look up an AVHWDeviceType by name. *
Add a function to get the corresponding AVHWDeviceType from a given hardware pixel format. Signed-off-by: Tong Wu <tong1.wu@intel.com> --- libavutil/hwcontext.c | 12 ++++++++++++ libavutil/hwcontext.h | 9 +++++++++ 2 files changed, 21 insertions(+)