media: coda: Fix H.264 header alignment.

Message ID 508e4a66.c25fb40a.02b1.ffffbb4e@mx.google.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Javier Martin Oct. 29, 2012, 9:20 a.m. UTC
  From: Javier Martin <javier.martin@vista-silicon.com>

Length of H.264 headers is variable and thus it might not be
aligned for the coda to append the encoded frame. This causes
the first frame to overwrite part of the H.264 PPS.

In order to solve that, a filler NAL must be added between
the headers and the first frame to preserve alignment.

Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
---
 drivers/media/platform/coda.c |   33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)
  

Comments

Philipp Zabel Oct. 29, 2012, 10:31 a.m. UTC | #1
Hi Javier,

Am Montag, den 29.10.2012, 10:20 +0100 schrieb
javier.martin@vista-silicon.com:
> From: Javier Martin <javier.martin@vista-silicon.com>
> 
> Length of H.264 headers is variable and thus it might not be
> aligned for the coda to append the encoded frame. This causes
> the first frame to overwrite part of the H.264 PPS.
> 
> In order to solve that, a filler NAL must be added between
> the headers and the first frame to preserve alignment.

I can reproduce this, but CODA7/9 even need 64-bit alignment.

> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
> ---
>  drivers/media/platform/coda.c |   33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> index a8c7a94..4cdb4f4 100644
> --- a/drivers/media/platform/coda.c
> +++ b/drivers/media/platform/coda.c
> @@ -177,6 +177,8 @@ struct coda_ctx {
>  	int				idx;
>  };
>  
> +static u8 coda_filler_nal[] = { 0x00, 0x00, 0x00, 0x01, 0x0c, 0xff, 0xff, 0xff, 0xff, 0xff};
> +
>  static inline void coda_write(struct coda_dev *dev, u32 data, u32 reg)
>  {
>  	v4l2_dbg(1, coda_debug, &dev->v4l2_dev,
> @@ -938,6 +940,29 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx, struct coda_q_data *q_d
>  	return 0;
>  }
>  
> +static int coda_h264_padding(int size, char *p)
> +{
> +	int size_align = size & ~0x3;
> +	int filler_size = ARRAY_SIZE(coda_filler_nal);
> +	int nal_size;
> +	int diff;
> +
> +	diff = size - size_align;
> +	if (diff == 0)
> +		return 0;
> +
> +	nal_size = filler_size + 2 - diff;
> +	if (nal_size > filler_size)
> +		nal_size -= 4;
> +
> +	memcpy(p, coda_filler_nal, nal_size);
> +
> +	/* Add rbsp stop bit and trailing at the end */
> +	*(p + nal_size - 1) = 0x80;
> +
> +	return nal_size;
> +}
> +
>  static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
>  {
>  	struct coda_ctx *ctx = vb2_get_drv_priv(q);
> @@ -1165,7 +1190,13 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
>  				coda_read(dev, CODA_CMD_ENC_HEADER_BB_START);
>  		memcpy(&ctx->vpu_header[1][0], vb2_plane_vaddr(buf, 0),
>  		       ctx->vpu_header_size[1]);
> -		ctx->vpu_header_size[2] = 0;
> +		/*
> +		 * Length of H.264 headers is variable and thus it might not be
> +		 * aligned for the coda to append the encoded frame. In that is
> +		 * the case a filler NAL must be added to header 2.
> +		 */
> +		ctx->vpu_header_size[2] = coda_h264_padding((ctx->vpu_header_size[0] +
> +					ctx->vpu_header_size[1]), ctx->vpu_header[2]);
>  		break;
>  	case V4L2_PIX_FMT_MPEG4:
>  		/*

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Patch

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index a8c7a94..4cdb4f4 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -177,6 +177,8 @@  struct coda_ctx {
 	int				idx;
 };
 
+static u8 coda_filler_nal[] = { 0x00, 0x00, 0x00, 0x01, 0x0c, 0xff, 0xff, 0xff, 0xff, 0xff};
+
 static inline void coda_write(struct coda_dev *dev, u32 data, u32 reg)
 {
 	v4l2_dbg(1, coda_debug, &dev->v4l2_dev,
@@ -938,6 +940,29 @@  static int coda_alloc_framebuffers(struct coda_ctx *ctx, struct coda_q_data *q_d
 	return 0;
 }
 
+static int coda_h264_padding(int size, char *p)
+{
+	int size_align = size & ~0x3;
+	int filler_size = ARRAY_SIZE(coda_filler_nal);
+	int nal_size;
+	int diff;
+
+	diff = size - size_align;
+	if (diff == 0)
+		return 0;
+
+	nal_size = filler_size + 2 - diff;
+	if (nal_size > filler_size)
+		nal_size -= 4;
+
+	memcpy(p, coda_filler_nal, nal_size);
+
+	/* Add rbsp stop bit and trailing at the end */
+	*(p + nal_size - 1) = 0x80;
+
+	return nal_size;
+}
+
 static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
 {
 	struct coda_ctx *ctx = vb2_get_drv_priv(q);
@@ -1165,7 +1190,13 @@  static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
 				coda_read(dev, CODA_CMD_ENC_HEADER_BB_START);
 		memcpy(&ctx->vpu_header[1][0], vb2_plane_vaddr(buf, 0),
 		       ctx->vpu_header_size[1]);
-		ctx->vpu_header_size[2] = 0;
+		/*
+		 * Length of H.264 headers is variable and thus it might not be
+		 * aligned for the coda to append the encoded frame. In that is
+		 * the case a filler NAL must be added to header 2.
+		 */
+		ctx->vpu_header_size[2] = coda_h264_padding((ctx->vpu_header_size[0] +
+					ctx->vpu_header_size[1]), ctx->vpu_header[2]);
 		break;
 	case V4L2_PIX_FMT_MPEG4:
 		/*