[Mesa-dev] [PATCH 00/20] MJPEG decode support through VA-API

[Christian König]

Hi Leo,

Patches #2, #4, #6, #8 - #12, #14, #18-#20 are Reviewed-by: Christian 
König <christian.koenig at amd.com>

Patch #1:

When you add new formats it would be nice to have 
u_reduce_video_profile() in src/gallium/auxiliary/util/u_video.h updated 
as well.

Additional to that I think the codec name is JPEG, not MJPEG. MJPEG is 
just the file/stream format with multiple JPEGs.

Patch #3:

> +		case PIPE_VIDEO_FORMAT_MJPEG:
> +			return true;
Please reorder the patch to be the last of the radeon/* patches, so that 
the driver only starts to claim JPEG support when everything else is 
implemented.

Apart from that the patch is Reviewed-by: Christian König 
<christian.koenig at amd.com>

Patch #5:
> +	if (dec->stream_type != RUVD_CODEC_MJPEG) {
> +		dpb_size = calc_dpb_size(dec);
> +		if (!rvid_create_buffer(dec->screen, &dec->dpb, dpb_size, PIPE_USAGE_DEFAULT)) {
> +			RVID_ERR("Can't allocated dpb.\n");
> +			goto error;
> +		}
> +		rvid_clear_buffer(context, &dec->dpb);
I think it would be cleaner if calc_dpb_size() just return zero for 
RUVD_CODEC_MJPEG and then the calling code skipping buffer allocation 
when it sees a zero sized dpb.

Patch #7:
> +      enum pipe_video_format format =
> +         u_reduce_video_profile(context->templat.profile);
> +
Here you use u_reduce_video_profile, but I can't see where you update 
u_reduce_video_profile() for JPEG support?

Patch #13:

Mhm, I'm not sure if it is a good idea to add this to the driver 
backend, but the patch is Acked-by: Christian König 
<christian.koenig at amd.com> anyway.

Patch #15, #16 and #17 are somehow missing and didn't made it to the 
mailing list.

Regards,
Christian.

Am 15.08.2017 um 22:08 schrieb Leo Liu:
> The series is able to enable mjpeg decode support through vaapi, and that
> includes for the formats of 420(NV12) and 422(YUYV).
>
> Leo Liu (20):
>    vl: add mjpeg profile and format
>    vl: add mjpeg picture description
>    radeon/video: add mjpeg support
>    radeon/uvd: add mjpeg stream type
>    radeon/uvd: add mjpeg support
>    st/va: add mjpeg picture to context
>    st/va: create decoder for mjpeg format
>    st/va: add handles for mjpeg Buffers
>    st/va: add picture parameter handling for mjpeg
>    st/va: add iq matrix handling for mjpeg
>    st/va: add huffman table handling for mjpeg
>    st/va: add slice parameter handling for mjpeg
>    radeon/uvd: reconstruct mjpeg bitstream
>    st/va: make surface allocate functions more usefully
>    radeon/video: mjpeg not support stacked video buffers
>    st/va: relocate surface when stack field false
>    radeon/uvd: add yuyv format support for target buffer
>    st/va: detect mjpeg format from bitstream
>    st/va: relocate surface with yuyv stream
>    st/va: add mjpeg for config
>
>   src/gallium/auxiliary/util/u_video.h           |   3 +
>   src/gallium/drivers/radeon/radeon_uvd.c        | 175 +++++++++++++++++++++++--
>   src/gallium/drivers/radeon/radeon_uvd.h        |   1 +
>   src/gallium/drivers/radeon/radeon_video.c      |   8 +-
>   src/gallium/drivers/radeonsi/si_uvd.c          |   2 +-
>   src/gallium/include/pipe/p_video_enums.h       |   6 +-
>   src/gallium/include/pipe/p_video_state.h       |  59 +++++++++
>   src/gallium/state_trackers/va/Makefile.sources |   1 +
>   src/gallium/state_trackers/va/config.c         |   2 +-
>   src/gallium/state_trackers/va/picture.c        |  70 +++++++++-
>   src/gallium/state_trackers/va/picture_mjpeg.c  | 116 ++++++++++++++++
>   src/gallium/state_trackers/va/surface.c        |   8 +-
>   src/gallium/state_trackers/va/va_private.h     |  14 ++
>   13 files changed, 440 insertions(+), 25 deletions(-)
>   create mode 100644 src/gallium/state_trackers/va/picture_mjpeg.c
>