[Mesa-dev] [PATCH 00/20] MJPEG decode support through VA-API
Leo Liu
leo.liu at amd.com
Wed Aug 16 17:49:09 UTC 2017
On 08/16/2017 01:40 PM, Leo Liu wrote:
>
>
> 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.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.
>
> 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.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.
>
> Sure in v2 patch3.
Sorry in v2 patch 4, because the small reorder here.
Leo
>
>>
>> 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.koenig at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list