[FFmpeg-devel] [PATCH]lavu/hwaccel_vaapi: Add a fixme for the missing byte_order check

[Mark Thompson]

On 05/10/16 13:06, Carl Eugen Hoyos wrote:
> ---
> Hi!
> 
> I cannot test here but afaict, the current implementation of 
> vaapi_pix_fmt_from_fourcc() can't be correct.
> 
> Please comment, Carl Eugen
>
>  libavutil/hwcontext_vaapi.c |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
> index 92fa235..3319839 100644
> --- a/libavutil/hwcontext_vaapi.c
> +++ b/libavutil/hwcontext_vaapi.c
> @@ -92,6 +92,7 @@ typedef struct VAAPISurfaceMap {
>      }
>  // The map fourcc <-> pix_fmt isn't bijective because of the annoying U/V
>  // plane swap cases.  The frame handling below tries to hide these.
> +// FIXME: Take VAImageFormat->byte_order into account
>  static struct {
>      unsigned int fourcc;
>      unsigned int rt_format;
> --
> 1.7.10.4

Hi Carl,

Have you found / had reported to you some case which causes a problem here?  If
so, please could you offer some more detail about it (especially the driver
being used).

In the cases I know of, the VAImageFormat structures are are all hard-coded such
that the fourcc is really the only meaningful information in them:

*
<https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/state_trackers/va/image.c#n41>
* <https://cgit.freedesktop.org/vaapi/intel-driver/tree/src/i965_drv_video.c#n246>

This is reflected by the use in hwcontext_vaapi.c, which only fetches the
driver's version of the structure in order to pass it back in vaCreateImage()
calls - the other fields are never touched at all.

So, I don't think this comment adds any value.

Thanks,

- Mark


PS:  Testing with mesa (or any other driver) on a big-endian machine would be
very welcome, if someone has the hardware to do it.