On 08/16/2017 03:22 AM, Christian König wrote:
Hi Leo,
Patches #2, #4, #6, #8 - #12, #14, #18-#20 are Reviewed-by: Christian
König <christian.koe...@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.
Yes. I have "u_reduce_video_profile()" updated in patch 3.
The patch1 and patch2 are for vl headers, but I will move
u_reduce_video_profile() to patch 1 in v2
Additional to that I think the codec name is JPEG, not MJPEG. MJPEG is
just the file/stream format with multiple JPEGs.
Okay.
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.
Sure in v2 patch 5, also add check for kernel version and ASICs.
Apart from that the patch is Reviewed-by: Christian König
<christian.koe...@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.
Sure in v2 patch3.
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?
It's at original patch 3.
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.koe...@amd.com> anyway.
Patch #15, #16 and #17 are somehow missing and didn't made it to the
mailing list.
>The subject line should probably read "reallocate" instead of "relocate".
>Additional to that the term "stacked field" isn't really used outside
AMD, better use interlaced layout.
I will fix commit message.
> case RUVD_SURFACE_TYPE_LEGACY:
> msg->body.decode.dt_pitch = luma->u.legacy.level[0].nblk_x;
>+ if (!chroma)
>+ msg->body.decode.dt_pitch *= 2;
>Patch looks good to me, except for this hunk.
>Can we get the pitch somehow else instead of making it depending if
chroma is present or not?
>Cause that is clearly not correct in all cases.
I will add stream type check in v2.
Thanks for the review.
Leo
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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
