Hi sebastian,
On 2025/2/24 22:17, Sebastian Fricke wrote:
[You don't often get email from sebastian.fricke@xxxxxxxxxxxxx. Learn
why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Hey Ming,
On 17.01.2025 16:57, Ming Qian wrote:
The amphion decoder will pre-parse 3 frames before decoding the first
frame. If we append a flush padding data after frame, the decoder
will finish parsing and start to decode when the flush data is parsed.
It can reduce the decoding latency.
In the past, we only enable this mode when the display delay is set to
0. But we still can enable this mode without changing the display order,
so we add a frame_flush_mode parameter to enable it.
My recommendation:
By default the amphion decoder will pre-parse 3 frames before starting
to decode the first frame. Alternatively, a block of flush padding data
can be appended to the frame, which will ensure that the decoder can
start decoding immediately after parsing the flush padding data, thus
potentially reducing decoding latency.
This mode was previously only enabled, when the display delay was set to
0. Allow the user to manually toggle the use of that mode via a module
parameter called frame_flush_mode, which enables the mode without
changing the display order.
Which fixes a few grammatical issues and tries to be a bit more clear.
But please confirm to me that I hit your intended meaning.
More comments below ...
Yes, you're right, and your description looks better, I will follow it.
Signed-off-by: Ming Qian <ming.qian@xxxxxxxxxxx>
---
drivers/media/platform/amphion/vpu_malone.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/amphion/vpu_malone.c
b/drivers/media/platform/amphion/vpu_malone.c
index 1d9e10d9bec1..f07660dc3b07 100644
--- a/drivers/media/platform/amphion/vpu_malone.c
+++ b/drivers/media/platform/amphion/vpu_malone.c
@@ -25,6 +25,9 @@
#include "vpu_imx8q.h"
#include "vpu_malone.h"
+static bool frame_flush_mode;
+module_param(frame_flush_mode, bool, 0644);
Could you add a comment here that makes clear to the reader briefly what
the expected behavior of frame_flush_mode = 0 and frame_flush_mode = 1
is?
OK, I'll add a comment to describe this mode.
+
#define CMD_SIZE 25600
#define MSG_SIZE 25600
#define CODEC_SIZE 0x1000
@@ -1579,7 +1582,7 @@ static int vpu_malone_input_frame_data(struct
vpu_malone_str_buffer __iomem *str
vpu_malone_update_wptr(str_buf, wptr);
- if (disp_imm && !vpu_vb_is_codecconfig(vbuf)) {
+ if ((disp_imm || frame_flush_mode) &&
!vpu_vb_is_codecconfig(vbuf)) {
So you say that the mode was enabled with display delay set to 0,
meaning (disp_imm = 1) == (display delay = 0), right? E.g. disp_imm
means display_immediately I guess.
I think this all deserves a lot better documentation, otherwise the code
becomes quite cryptic. Could you add a comment before this line, which
explains the entry conditions disp_imm & frame_flush_mode and the
codeconfig thing and that explains briefly what kind of mode we are
entering here?
Sure,I'll add comments to explain the code. I'm sorry fot the confusion.
Thank you very much for your feedback, I will try to fix them in v3 patch.
Thanks,
Ming
ret = vpu_malone_add_scode(inst->core->iface,
inst->id,
&inst->stream_buffer,
--
2.43.0-rc1
Regards,
Sebastian Fricke