Skip to contentSkip to conversation

s5p-mfc: encoder could free buffers

PATCH In review ayaka 2 participants 2 messages Jun 26, 2014 – Jul 3, 2014 Media (V4L/DVB)

lore
Share & tools
lore

ayaka

author
#0 lore raw

PATCH 1/1 s5p-mfc: encoder could free buffers

[PATCH] s5p-mfc: encoder could free buffers

The patch is necessary or the buffers could be freeed but it would break the state of encoder in s5p-mfc. It is also need by some application which would detect the buffer allocation way, like gstreamer.
---
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 8 ++++++++

1 file changed, 8 insertions(+)
(quoted diff) +0 -0

b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index d26b248..1a7518f 100644

drivers/media/platform/s5p-mfc/s5p_mfc_enc.c +0 -0
@@ -1166,6 +1166,10 @@ static int vidioc_reqbufs(struct file *file, void

*priv,
mfc_err("error in vb2_reqbufs() for E(D)\n");
return ret;
}
+ if (reqbufs->count == 0) {
+ mfc_debug(2, "Freeing buffers\n");
+ return ret;
+ }
ctx->capture_state = QUEUE_BUFS_REQUESTED;

ret = s5p_mfc_hw_call(ctx->dev->mfc_ops,
@@ -1200,6 +1204,10 @@ static int vidioc_reqbufs(struct file *file, void *priv,
mfc_err("error in vb2_reqbufs() for E(S)\n");
return ret;
}
+ if (reqbufs->count == 0) {
+ mfc_debug(2, "Freeing buffers\n");
+ return ret;
+ }
ctx->output_state = QUEUE_BUFS_REQUESTED;
} else {
mfc_err("invalid buf type\n");

Signature · 1 line
1.9.1
#1 lore raw

Hi Ayaka,

Thank you for your patch, however I see some things that need to be corrected.

Quoted 9 lines

From: [email protected] [mailto:linux-media- [email protected]] On Behalf Of ayaka
Sent: Thursday, June 26, 2014 4:15 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]
Subject: [PATCH] s5p-mfc: encoder could free buffers

[PATCH] s5p-mfc: encoder could free buffers

The above line is not necessary.

Quoted 2 lines

The patch is necessary or the buffers could be freeed but it would

Typo - should be "freed"

Quoted 3 lines

break the state of encoder in s5p-mfc. It is also need by some application which would detect the buffer allocation way, like gstreamer.

I would rephrase the whole commit message. Maybe something along the lines "Add handling of buffer freeing reqbufs request to the encoder of s5p-mfc."

Quoted 10 lines

---
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 8 ++++++++

1 file changed, 8 insertions(+)
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c +0 -0
index d26b248..1a7518f 100644

drivers/media/platform/s5p-mfc/s5p_mfc_enc.c static int vidioc_reqbufs(struct file *file, · line 1170

Quoted 5 lines
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c +4 -0
@@ -1166,6 +1166,10 @@ static int vidioc_reqbufs(struct file *file,
 void *priv, mfc_err("error in vb2_reqbufs() for E(D)\n"); return ret; }
+ if (reqbufs->count == 0) {
+ mfc_debug(2, "Freeing buffers\n");
+ return ret;
+ }

Indentation is wrong here, please correct.
Also, I think you should set ctx->capture_state to QUEUE_FREE.

drivers/media/platform/s5p-mfc/s5p_mfc_enc.c static int vidioc_reqbufs(struct file *file, · line 1208

Quoted 9 lines
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c +4 -0
 ctx->capture_state = QUEUE_BUFS_REQUESTED;
 
 ret = s5p_mfc_hw_call(ctx->dev->mfc_ops,

@@ -1200,6 +1204,10 @@ static int vidioc_reqbufs(struct file *file,
 void *priv, mfc_err("error in vb2_reqbufs() for E(S)\n"); return ret; }
+ if (reqbufs->count == 0) {
+ mfc_debug(2, "Freeing buffers\n");
+ return ret;
+ }

Indentation is wrong here, please correct.
Also, I think you should set ctx->capture_state to QUEUE_FREE.

drivers/media/platform/s5p-mfc/s5p_mfc_enc.c static int vidioc_reqbufs(struct file *file, · line 1211

Quoted 3 lines
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c +0 -0
 ctx->output_state = QUEUE_BUFS_REQUESTED;
 } else {
 mfc_err("invalid buf type\n");

In addition to the above, for capture this patch will not work. The following
check will prevent buffers from being freed:

if (ctx->capture_state != QUEUE_FREE) {
         mfc_err("invalid capture state: %d\n",
                                         ctx->capture_state);
         return -EINVAL;

}

Also, you should provide a proper Signed-off-by with your full name and email
address. Did you use git send-email to send this patch?

Best wishes,

Signature · 2 lines
Kamil Debski
Samsung R&D Institute Poland