Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   =?UTF-8?q?N=C3=ADcolas=20F=2E=20R=2E=20A=2E=20Prado?= 
        <nfraprado@collabora.com>
To:     Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc:     kernel@collabora.com,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        =?UTF-8?q?N=C3=ADcolas=20F=2E=20R=2E=20A=2E=20Prado?= 
        <nfraprado@collabora.com>,
        Andrew-CT Chen <andrew-ct.chen@mediatek.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Tiffany Lin <tiffany.lin@mediatek.com>,
        Yunfei Dong <yunfei.dong@mediatek.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-media@vger.kernel.org, linux-mediatek@lists.infradead.org
Subject: [PATCH] Revert "media: mediatek: vcodec: Fix bitstream crop information error"
Date:   Tue, 18 Apr 2023 17:50:52 -0400
Message-Id: <20230418215052.2371755-1-nfraprado@collabora.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

This reverts commit cd61f3c6794bea2b717fe6083ca2ad189db75418. That
commit's purpose was to prevent the padding from being decoded when
userspace didn't set a selection, relying on the default one.

However, as described in the Step 6 of the Initialization procedure for
the Memory-to-Memory Stateful Video Encoder Interface [1]:

"Set the visible resolution for the stream metadata via
VIDIOC_S_SELECTION() on the OUTPUT queue if it is desired to be
different than the full OUTPUT resolution."

And from the Note:

"To avoid encoding the padding, the client needs to explicitly configure
this selection target"

Hence the behavior in the original commit doesn't follow the interface
and needs to be reverted.

This fixes the following v4l2-compliance failure observed on
mt8192-asurada-spherion:

		fail: v4l2-test-formats.cpp(924): sel.r.width != fmt.g_width()
	test VIDIOC_S_FMT: FAIL

[1] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/dev-encoder.html#initialization

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

---
Hi,

With this patch applied, there's only one failure left on
mt8192-asurada-spherion:

                fail: v4l2-test-controls.cpp(1167): node->codec_mask & STATEFUL_ENCODER
        test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL

But from the documentation I see that the V4L2_EVENT_EOS for stateful
encoder is deprecated behavior, so should the v4l2-compliance test be
the one changed to not require this?

We're enabling running v4l2-compliance with this driver in KernelCI [2],
so after we fix these two remaining issues it should be easy to notice
if any failures are introduced.

Thanks,
Nícolas

[2] https://github.com/kernelci/kernelci-core/pull/1889

 drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
index db65e77bd373..9ff439a50f53 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
@@ -505,13 +505,13 @@ static int vidioc_venc_s_fmt_out(struct file *file, void *priv,
 		f->fmt.pix.pixelformat = fmt->fourcc;
 	}
 
-	q_data->visible_width = f->fmt.pix_mp.width;
-	q_data->visible_height = f->fmt.pix_mp.height;
-	q_data->fmt = fmt;
-	ret = vidioc_try_fmt_out(ctx, f, q_data->fmt);
+	ret = vidioc_try_fmt_out(ctx, f, fmt);
 	if (ret)
 		return ret;
 
+	q_data->fmt = fmt;
+	q_data->visible_width = f->fmt.pix_mp.width;
+	q_data->visible_height = f->fmt.pix_mp.height;
 	q_data->coded_width = f->fmt.pix_mp.width;
 	q_data->coded_height = f->fmt.pix_mp.height;
 
-- 
2.40.0