[FFmpeg-devel] [PATCH] avcodec: v4l2_m2m: add MJPEG enc/dec support
Maxime Jourdan
maxi.jourdan at wanadoo.fr
Tue Aug 14 06:06:27 EEST 2018
2018-08-13 2:26 GMT+02:00 Mark Thompson <sw at jkqxz.net>:
> 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.
Sounds good
>>> 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 ]---
Thanks for looking, unfortunately those dmesg traces don't bring much
more to the table :< .
>>> 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.)
I agree, but this is how it's done in V4L2 as well: everything must be
set prior to decoding. I think the most crucial aspect is that you
need to have the buffers - with the correct size/planes, which depend
on the pixfmt - ready before decoding.
So, some code will be needed to match the jpeg's pixfmt to the
driver's CAPTURE pixfmt.
>>>
>>> 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.)
>
Even on the C2 it's not perfect. For instance, I tried to feed 4:2:2
MJPEG to the decoder and it produced garbage: turns out it only
supports yuv420p. I'm thinking that ffmpeg shouldn't even try to
decode with v4l2 if the jpeg's pixfmt isn't in the CAPTURE pixfmt
list.
And in case the driver supports up/downsampling, well that's too bad
because we won't be able to know.
Btw, this issue is not specific to MJPEG. Some HW decoders can only do
H.264 4:2:0 8-bit, but in the current code ffmpeg will happily send
H.264 4:2:2 10-bit to a v4l2 decoder that doesn't support it. You can
only pray that the HW decoder has some kind of failsafe to abort
decoding if it doesn't support the input.
I need to think more about this, but on the V4L2 side we should have
fine-tuned input parameters that allow specifying more than just the
compression standard.
> Thanks,
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Maxime
More information about the ffmpeg-devel
mailing list