diff mbox

[FFmpeg-devel] avcodec/v4l2_m2m_enc: Check encoder pix_fmt matches pix_fmt on device

Message ID 20191110194210.18010-1-andriy.gelman@gmail.com
State Superseded
Headers show

Commit Message

Andriy Gelman Nov. 10, 2019, 7:42 p.m. UTC 
From: Andriy Gelman <andriy.gelman@gmail.com>

Fixes #8079

During initialization of a v4l2m2m device, the configured pix_fmt can be
different to the pix_fmt of the encoder (i.e. avctx->pix_fmt).

For example on the Odroid XU4:
./ffmpeg -f lavfi -i yuvtestsrc -codec:v h264_v4l2m2m out.h264

will configure the v4l2 encoder to pix_fmt nv21, whereas the input
frames will be yuv444p.

This commit checks that the configured v4l2 pix_fmt on device is the
same as avctx->pix_fmt. If they are different it returns an error and
suggests a cli input that will fix the problem. This has been tested on
RPI4 and Odroid XU4.

Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
---
 libavcodec/v4l2_m2m_enc.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Carl Eugen Hoyos Nov. 10, 2019, 10:05 p.m. UTC | #1 
> Am 11.11.2019 um 04:42 schrieb Andriy Gelman <andriy.gelman@gmail.com>:
> 
> +    /* check configured pix_fmt matches avctx->pix_fmt */

Is this comment really useful?

> +    pix_fmt_output = ff_v4l2_format_v4l2_to_avfmt(v4l2_fmt_output, AV_CODEC_ID_RAWVIDEO);
> +    if (pix_fmt_output != avctx->pix_fmt) {
> +        const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt_output);
> +        av_log(priv, AV_LOG_ERROR, "Pixel format not configured. Encoder requires %s "


> +                                   " pixel format. On command line add setting:"
> +                                   " -pix_fmt %s to force correct pixel format.\n",

This should be rarely printed by a library, I don’t think this is a good exception.

Thank you for looking into this issue, Carl Eugen
Andriy Gelman Nov. 12, 2019, 3:56 a.m. UTC | #2 
On Mon, 11. Nov 07:05, Carl Eugen Hoyos wrote:
> 
> 
> > Am 11.11.2019 um 04:42 schrieb Andriy Gelman <andriy.gelman@gmail.com>:
> > 
> > +    /* check configured pix_fmt matches avctx->pix_fmt */
> 
> Is this comment really useful?

ok, will remove

> 
> > +    pix_fmt_output = ff_v4l2_format_v4l2_to_avfmt(v4l2_fmt_output, AV_CODEC_ID_RAWVIDEO);
> > +    if (pix_fmt_output != avctx->pix_fmt) {
> > +        const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt_output);
> > +        av_log(priv, AV_LOG_ERROR, "Pixel format not configured. Encoder requires %s "
> 
> 
> > +                                   " pixel format. On command line add setting:"
> > +                                   " -pix_fmt %s to force correct pixel format.\n",
> 
> This should be rarely printed by a library, I don’t think this is a good exception.

I thought it would be useful to tell the user how to solve the problem. 

I can modify the docs instead and add option -list_pix_fmt
that tells the user what pixel formats are supported?
diff mbox

Patch

diff --git a/libavcodec/v4l2_m2m_enc.c b/libavcodec/v4l2_m2m_enc.c
index 474e6bef89..5c8d49bd2c 100644
--- a/libavcodec/v4l2_m2m_enc.c
+++ b/libavcodec/v4l2_m2m_enc.c
@@ -30,6 +30,7 @@ 
 #include "libavutil/opt.h"
 #include "v4l2_context.h"
 #include "v4l2_m2m.h"
+#include "v4l2_fmt.h"
 
 #define MPEG_CID(x) V4L2_CID_MPEG_VIDEO_##x
 #define MPEG_VIDEO(x) V4L2_MPEG_VIDEO_##x
@@ -288,6 +289,8 @@  static av_cold int v4l2_encode_init(AVCodecContext *avctx)
     V4L2Context *capture, *output;
     V4L2m2mContext *s;
     V4L2m2mPriv *priv = avctx->priv_data;
+    enum AVPixelFormat pix_fmt_output;
+    uint32_t v4l2_fmt_output;
     int ret;
 
     ret = ff_v4l2_m2m_create_context(priv, &s);
@@ -316,6 +319,22 @@  static av_cold int v4l2_encode_init(AVCodecContext *avctx)
     }
     s->avctx = avctx;
 
+    if (V4L2_TYPE_IS_MULTIPLANAR(output->type))
+        v4l2_fmt_output = output->format.fmt.pix_mp.pixelformat;
+    else
+        v4l2_fmt_output = output->format.fmt.pix.pixelformat;
+
+    /* check configured pix_fmt matches avctx->pix_fmt */
+    pix_fmt_output = ff_v4l2_format_v4l2_to_avfmt(v4l2_fmt_output, AV_CODEC_ID_RAWVIDEO);
+    if (pix_fmt_output != avctx->pix_fmt) {
+        const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt_output);
+        av_log(priv, AV_LOG_ERROR, "Pixel format not configured. Encoder requires %s "
+                                   " pixel format. On command line add setting:"
+                                   " -pix_fmt %s to force correct pixel format.\n",
+                                   av_fourcc2str(v4l2_fmt_output), desc->name);
+        return AVERROR(EINVAL);
+    }
+
     return v4l2_prepare_encoder(s);
 }