On Sat, Feb 15, 2014 at 2:30 PM, Emil Velikov <emil.l.veli...@gmail.com> wrote:
> The nv84 code (vp2)
> - bitstream h264
> - idct and bitstream mpeg12
> Generic video (vpe2)
> - mc and idct mpeg12
>
> Signed-off-by: Emil Velikov <emil.l.veli...@gmail.com>
> ---
>
> Seems like some explicit checks would be more robust when
> new code is added. Better to deny support for the user than
> crash of feed junk data to the gpu.
>
> src/gallium/drivers/nouveau/nouveau_video.c | 12 ++++++++----
> src/gallium/drivers/nouveau/nv50/nv84_video.c | 20 +++++++++++++++-----
> 2 files changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nouveau_video.c
> b/src/gallium/drivers/nouveau/nouveau_video.c
> index 8795c9d..9c4711b 100644
> --- a/src/gallium/drivers/nouveau/nouveau_video.c
> +++ b/src/gallium/drivers/nouveau/nouveau_video.c
> @@ -510,8 +510,7 @@ nouveau_create_decoder(struct pipe_context *context,
> int ret;
> bool is8274 = screen->device->chipset > 0x80;
>
> - debug_printf("Acceleration level: %s\n", templ->entrypoint <=
> PIPE_VIDEO_ENTRYPOINT_BITSTREAM ? "bit":
> - templ->entrypoint ==
> PIPE_VIDEO_ENTRYPOINT_IDCT ? "IDCT" : "MC");
> + debug_printf("Acceleration level: %s\n", templ->entrypoint ==
> PIPE_VIDEO_ENTRYPOINT_IDCT ? "IDCT" : "MC");
>
> if (getenv("XVMC_VL"))
> goto vl;
> @@ -838,8 +837,13 @@ nouveau_screen_get_video_param(struct pipe_screen
> *pscreen,
> {
> switch (param) {
> case PIPE_VIDEO_CAP_SUPPORTED:
> - return entrypoint >= PIPE_VIDEO_ENTRYPOINT_IDCT &&
> - u_reduce_video_profile(profile) == PIPE_VIDEO_FORMAT_MPEG12;
> + switch (entrypoint) {
> + case PIPE_VIDEO_ENTRYPOINT_IDCT:
> + case PIPE_VIDEO_ENTRYPOINT_MC:
> + return u_reduce_video_profile(profile) == PIPE_VIDEO_FORMAT_MPEG12;
> + default:
> + return 0;
> + }
> case PIPE_VIDEO_CAP_NPOT_TEXTURES:
> return 1;
> case PIPE_VIDEO_CAP_MAX_WIDTH:
> diff --git a/src/gallium/drivers/nouveau/nv50/nv84_video.c
> b/src/gallium/drivers/nouveau/nv50/nv84_video.c
> index a39f572..d0a3580 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv84_video.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv84_video.c
> @@ -281,8 +281,9 @@ nv84_create_decoder(struct pipe_context *context,
> return vl_create_decoder(context, templ);
>
> if ((is_h264 && templ->entrypoint != PIPE_VIDEO_ENTRYPOINT_BITSTREAM) ||
> - (is_mpeg12 && templ->entrypoint > PIPE_VIDEO_ENTRYPOINT_IDCT)) {
> - debug_printf("%x\n", templ->entrypoint);
> + (is_mpeg12 && (templ->entrypoint != PIPE_VIDEO_ENTRYPOINT_BITSTREAM ||
> + templ->entrypoint != PIPE_VIDEO_ENTRYPOINT_IDCT))) {
> + debug_printf("invalid entrypoint %x\n", templ->entrypoint);
This code was added before the format checks had an entrypoint in
them. I think this whole if statement can safely be removed now -- the
expectation is that gallium callers perform the required checks
(except that it wasn't possible before, hence this logic). But if you
_really_ want to have the check here, may as well flip it to the
switch style.
> return NULL;
> }
>
> @@ -812,9 +813,18 @@ nv84_screen_get_video_param(struct pipe_screen *pscreen,
> switch (param) {
> case PIPE_VIDEO_CAP_SUPPORTED:
> codec = u_reduce_video_profile(profile);
> - return (codec == PIPE_VIDEO_FORMAT_MPEG4_AVC ||
> - codec == PIPE_VIDEO_FORMAT_MPEG12) &&
> - firmware_present(pscreen, codec);
> + switch (codec) {
> + case PIPE_VIDEO_FORMAT_MPEG12:
> + return ((entrypoint == PIPE_VIDEO_ENTRYPOINT_BITSTREAM) ||
> + (entrypoint == PIPE_VIDEO_ENTRYPOINT_IDCT)) &&
You don't need the extra parens around a == b. (Same below.)
> + firmware_present(pscreen, codec);
> + case PIPE_VIDEO_FORMAT_MPEG4_AVC:
> + return (entrypoint == PIPE_VIDEO_ENTRYPOINT_BITSTREAM) &&
> + firmware_present(pscreen, codec);
> + default:
> + return 0;
> + }
> +
> case PIPE_VIDEO_CAP_NPOT_TEXTURES:
> return 1;
> case PIPE_VIDEO_CAP_MAX_WIDTH:
> --
> 1.8.5.5
With the above small changes,
Reviewed-by: Ilia Mirkin <imir...@alum.mit.edu>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
