[FFmpeg-devel] [PATCH] avcodec: v4l2_m2m: add MJPEG enc/dec support
Mark Thompson
sw at jkqxz.net
Mon Aug 13 03:26:32 EEST 2018
On 13/08/18 00:33, Maxime Jourdan wrote:
> Hi Jorge, unfortunately don't have a SBC that features a V4L2 M2M
> MJPEG/JPEG encoder so I couldn't test it.
>
> Hi Mark, thanks for the extensive review. Too bad neither scenario worked fully.
>
> 2018-08-12 19:24 GMT+02:00 Mark Thompson <sw at jkqxz.net>:
>> On 12/08/18 15:40, Maxime Jourdan wrote:
>>> Tested on an Odroid-C2 with a V4L2 M2M MJPEG decoder.
>>>
>>> Signed-off-by: Maxime Jourdan <maxi.jourdan at wanadoo.fr>
>>> ---
>>> configure | 3 +++
>>> libavcodec/Makefile | 2 ++
>>> libavcodec/allcodecs.c | 2 ++
>>> libavcodec/v4l2_m2m_dec.c | 1 +
>>> libavcodec/v4l2_m2m_enc.c | 1 +
>>> 5 files changed, 9 insertions(+)
>>>
>>> ...
>>> diff --git a/libavcodec/v4l2_m2m_enc.c b/libavcodec/v4l2_m2m_enc.c
>>> index 4c9ea1fd92..b025f59275 100644
>>> --- a/libavcodec/v4l2_m2m_enc.c
>>> +++ b/libavcodec/v4l2_m2m_enc.c
>>> @@ -345,6 +345,7 @@ AVCodec ff_ ## NAME ## _v4l2m2m_encoder = { \
>>> .wrapper_name = "v4l2m2m", \
>>> };
>>>
>>> +M2MENC(mjpeg,"MJPEG", AV_CODEC_ID_MJPEG);
>>> M2MENC(mpeg4,"MPEG4", AV_CODEC_ID_MPEG4);
>>> M2MENC(h263, "H.263", AV_CODEC_ID_H263);
>>> M2MENC(h264, "H.264", AV_CODEC_ID_H264);
>>
>> Since MJPEG is intra-only, it probably wants to avoid setting options like GOP size and B-frames (currently this gives me warnings for each of these options that it failed to set them, visible in log below).
>
> Ack, although right now I'm not sure what's the cleanest way to enforce this.
I guess make an intra-only flag based on the codec and then add "if (!intra-only)" around applying those settings in the encoder.
>> Trying this on an Odroid XU4 (Exynos S5P running Linux 4.14.55), it looks like we need to somehow allow both "MJPG" and "JPEG" as fourccs. Reordering the two lines in libavcodec/v4l2_fmt.c to put "JPEG" first makes it get past that test, but obviously that will stop it from working on an "MJPG" device.
>
> I see, so some rework will be needed to account for AV_CODECs that
> appear multiple times in the list.
Yeah, probably. (Assuming there isn't some special difference between them which I have missed.)
>>
>> So, for testing I used this patch:
>>
>> diff --git a/libavcodec/v4l2_fmt.c b/libavcodec/v4l2_fmt.c
>> index 6df47e3..06b57db 100644
>> --- a/libavcodec/v4l2_fmt.c
>> +++ b/libavcodec/v4l2_fmt.c
>> @@ -51,8 +51,8 @@ static const struct fmt_conversion {
>> { AV_FMT(YUV410P), AV_CODEC(RAWVIDEO), V4L2_FMT(YUV410) },
>> { AV_FMT(YUV410P), AV_CODEC(RAWVIDEO), V4L2_FMT(YVU410) },
>> { AV_FMT(NV12), AV_CODEC(RAWVIDEO), V4L2_FMT(NV12) },
>> - { AV_FMT(NONE), AV_CODEC(MJPEG), V4L2_FMT(MJPEG) },
>> { AV_FMT(NONE), AV_CODEC(MJPEG), V4L2_FMT(JPEG) },
>> + { AV_FMT(NONE), AV_CODEC(MJPEG), V4L2_FMT(MJPEG) },
>> #ifdef V4L2_PIX_FMT_SRGGB8
>> { AV_FMT(BAYER_BGGR8), AV_CODEC(RAWVIDEO), V4L2_FMT(SBGGR8) },
>> { AV_FMT(BAYER_GBRG8), AV_CODEC(RAWVIDEO), V4L2_FMT(SGBRG8) },
>>
>>
>> After getting past that, the decoder seems to hang for me pretty much immediately on a DQBUF call.
>
> Did dmesg have anything interesting to say ?
Huh, I did not think to look there.
Running again, I get the following when SIGINTing to kill the decoder (nothing before that):
[ 196.346037] ------------[ cut here ]------------
[ 196.346137] WARNING: CPU: 0 PID: 972 at drivers/media/v4l2-core/videobuf2-core.c:1655 __vb2_queue_cancel+0x17c/0x1e4
[ 196.346154] Modules linked in: cpufreq_conservative cpufreq_userspace cpufreq_powersave spidev spi_s3c64xx evdev gpio_keys uio_pdrv_genirq uio extcon_usb_gpio exynos_gpiomem uvcvideo snd_usb_audio snd_hwdep snd_usbmidi_lib videobuf2_vmalloc snd_rawmidi snd_seq_device ipv6
[ 196.346424] CPU: 0 PID: 972 Comm: ffmpeg_g Tainted: G W 4.14.55-133 #1
[ 196.346441] Hardware name: ODROID-XU4
[ 196.346522] [<c0110b14>] (unwind_backtrace) from [<c010ce98>] (show_stack+0x10/0x14)
[ 196.346585] [<c010ce98>] (show_stack) from [<c08a9c94>] (dump_stack+0x84/0x98)
[ 196.346655] [<c08a9c94>] (dump_stack) from [<c01242cc>] (__warn+0xec/0x104)
[ 196.346717] [<c01242cc>] (__warn) from [<c0124394>] (warn_slowpath_null+0x20/0x28)
[ 196.346780] [<c0124394>] (warn_slowpath_null) from [<c06adf14>] (__vb2_queue_cancel+0x17c/0x1e4)
[ 196.346846] [<c06adf14>] (__vb2_queue_cancel) from [<c06ae5d4>] (vb2_core_queue_release+0x18/0x38)
[ 196.346906] [<c06ae5d4>] (vb2_core_queue_release) from [<c06abc84>] (v4l2_m2m_ctx_release+0x1c/0x28)
[ 196.346969] [<c06abc84>] (v4l2_m2m_ctx_release) from [<c06cb514>] (s5p_jpeg_release+0x28/0x58)
[ 196.347030] [<c06cb514>] (s5p_jpeg_release) from [<c069716c>] (v4l2_release+0x38/0x74)
[ 196.347096] [<c069716c>] (v4l2_release) from [<c0286704>] (__fput+0x84/0x1cc)
[ 196.347156] [<c0286704>] (__fput) from [<c0141d40>] (task_work_run+0x90/0xb0)
[ 196.347209] [<c0141d40>] (task_work_run) from [<c01291cc>] (do_exit+0x390/0xb50)
[ 196.347259] [<c01291cc>] (do_exit) from [<c01299f4>] (do_group_exit+0x3c/0xbc)
[ 196.347310] [<c01299f4>] (do_group_exit) from [<c01342c8>] (get_signal+0x300/0x678)
[ 196.347373] [<c01342c8>] (get_signal) from [<c010bfb8>] (do_signal+0x64/0x430)
[ 196.347433] [<c010bfb8>] (do_signal) from [<c010c540>] (do_work_pending+0xac/0xc4)
[ 196.347491] [<c010c540>] (do_work_pending) from [<c01087ec>] (slow_work_pending+0xc/0x20)
[ 196.348088] ---[ end trace a4a2a65a0e7292e0 ]---
>> Input from a UVC webcam (Logitech C920, making 640x480 YUV 4:2:2 JPEGs):
>>
>> $ gdb --args ./ffmpeg_g -y -v 55 -f v4l2 -format mjpeg -framerate 30 -video_size 640x480 -c:v mjpeg_v4l2m2m -i /dev/video0 -f null -
>> ...
>>
>>
>> Input from a file (made with "./ffmpeg_g -y -f lavfi -i 'smptebars=size=640x480:rate=30' -an -pix_fmt yuvj420p -c:v mjpeg -frames:v 10 test.mp4"):
>>
>> $ gdb --args ./ffmpeg_g -v 55 -c:v mjpeg_v4l2m2m -i test.mp4 -f null -
>> ...
>>
>>
>> For the second run it looks suspicious that it's making a YUYV (4:2:2) output from a YUV420P (4:2:0) input file, but given that it fails in the same way as the first one I'm not sure that's actually relevant. Is there anything else I should be trying here?
>
> AFAIK there is currently no way to select the CAPTURE pixfmt, so the
> current code keeps the default format returned by the driver from
> G_FMT. In this case s5p-jpeg sets YUYV as default, which explains
> that.
>
> I think we'll need a -capture_pixfmt option to try setting it
> ourselves, unless there's a way to retrieve the -pix_fmt ffmpeg output
> option to try and match it ?
If the device can tell us what the output format should be then a decoder can just output that pixfmt.
The problem comes when it can't actually tell us in advance, but does offer a list of possible formats: it seems likely that only one of those choices will actually work - the YUV 4:2:0 JPEGs probably won't upsample into a YUYV output buffer, rather you need to pick the right output format for them. That might require parsing some of the input stream to decide which possible output formats actually match the input. Maybe it's similar to how the VAAPI hwaccel works for JPEG - the Intel driver offers a set of possible formats, but you need to pick the one which has the same chroma subsampling as your input stream for it to actually work.
(Pushing the choice onto the user as an extra option doesn't seem right to me.)
>>
>> The encoder does work, but seems to have a similar problem where it hangs at end-of-file:
>>
>> $ gdb --args ./ffmpeg_g -y -v 55 -f lavfi -i 'smptebars=size=640x480:rate=30' -an -pix_fmt yuyv422 -c:v mjpeg_v4l2m2m -frames:v 10 out.mjpeg
Encoder kernel outout:
The format test produces some spam when the encoder is starting:
[ 678.815745] vidioc_try_fmt:1070: failed to try output format
[ 678.820253] vidioc_try_fmt:1070: failed to try output format
[ 678.830969] vidioc_try_fmt:387: Unsupported format for source.
[ 678.835371] vidioc_try_fmt:387: Unsupported format for source.
[ 678.841224] vidioc_try_fmt:387: Unsupported format for source.
[ 678.847002] vidioc_try_fmt:387: Unsupported format for source.
[ 678.852840] vidioc_try_fmt:387: Unsupported format for source.
[ 678.858609] vidioc_try_fmt:387: Unsupported format for source.
[ 678.864452] vidioc_try_fmt:387: Unsupported format for source.
[ 678.870227] vidioc_try_fmt:387: Unsupported format for source.
[ 678.876061] vidioc_try_fmt:387: Unsupported format for source.
[ 678.881838] vidioc_try_fmt:387: Unsupported format for source.
[ 678.887678] vidioc_try_fmt:387: Unsupported format for source.
[ 678.894671] s5p-jpeg 11f60000.jpeg: Fourcc format (0x31324d4e) invalid.
And then we get a similar backtrace to the decoder when interrupting it:
[ 689.648208] ------------[ cut here ]------------
[ 689.648237] WARNING: CPU: 4 PID: 995 at drivers/media/v4l2-core/videobuf2-core.c:1655 __vb2_queue_cancel+0x17c/0x1e4
[ 689.648243] Modules linked in: cpufreq_conservative cpufreq_userspace cpufreq_powersave spidev spi_s3c64xx evdev gpio_keys uio_pdrv_genirq uio extcon_usb_gpio exynos_gpiomem uvcvideo snd_usb_audio snd_hwdep snd_usbmidi_lib videobuf2_vmalloc snd_rawmidi snd_seq_device ipv6
[ 689.648356] CPU: 4 PID: 995 Comm: ffmpeg_g Tainted: G W 4.14.55-133 #1
[ 689.648363] Hardware name: ODROID-XU4
[ 689.648384] [<c0110b14>] (unwind_backtrace) from [<c010ce98>] (show_stack+0x10/0x14)
[ 689.648400] [<c010ce98>] (show_stack) from [<c08a9c94>] (dump_stack+0x84/0x98)
[ 689.648417] [<c08a9c94>] (dump_stack) from [<c01242cc>] (__warn+0xec/0x104)
[ 689.648432] [<c01242cc>] (__warn) from [<c0124394>] (warn_slowpath_null+0x20/0x28)
[ 689.648447] [<c0124394>] (warn_slowpath_null) from [<c06adf14>] (__vb2_queue_cancel+0x17c/0x1e4)
[ 689.648462] [<c06adf14>] (__vb2_queue_cancel) from [<c06ae5d4>] (vb2_core_queue_release+0x18/0x38)
[ 689.648475] [<c06ae5d4>] (vb2_core_queue_release) from [<c06abc7c>] (v4l2_m2m_ctx_release+0x14/0x28)
[ 689.648490] [<c06abc7c>] (v4l2_m2m_ctx_release) from [<c06cb514>] (s5p_jpeg_release+0x28/0x58)
[ 689.648503] [<c06cb514>] (s5p_jpeg_release) from [<c069716c>] (v4l2_release+0x38/0x74)
[ 689.648518] [<c069716c>] (v4l2_release) from [<c0286704>] (__fput+0x84/0x1cc)
[ 689.648533] [<c0286704>] (__fput) from [<c0141d40>] (task_work_run+0x90/0xb0)
[ 689.648545] [<c0141d40>] (task_work_run) from [<c01291cc>] (do_exit+0x390/0xb50)
[ 689.648556] [<c01291cc>] (do_exit) from [<c01299f4>] (do_group_exit+0x3c/0xbc)
[ 689.648568] [<c01299f4>] (do_group_exit) from [<c01342c8>] (get_signal+0x300/0x678)
[ 689.648582] [<c01342c8>] (get_signal) from [<c010bfb8>] (do_signal+0x64/0x430)
[ 689.648595] [<c010bfb8>] (do_signal) from [<c010c540>] (do_work_pending+0xac/0xc4)
[ 689.648608] [<c010c540>] (do_work_pending) from [<c01087ec>] (slow_work_pending+0xc/0x20)
[ 689.648617] ---[ end trace a4a2a65a0e7292e1 ]---
>> The output file is correct here, though it's truncated because libavformat doesn't get an opportunity to finish it. (I had to give it a YUYV input explicitly - the output is messed up with another input format, but I don't think that's related to this patch.)
>
> I've glanced at the s5p-jpeg driver and I haven't seen any code to
> handle EOS (things like CMD_STOP or an empty input buffer), so I'm
> actually curious as to how they usually handle EOS with this driver.
(Note that it's not required that it works on this particular hardware to apply the patch, but it may well indicate problems and cause confusion later if it doesn't. It's definitely a good idea to see it working on more than one implementation, though.)
Thanks,
- Mark
More information about the ffmpeg-devel
mailing list