[FFmpeg-devel] [PATCH v4 4/4] fate/jpegxl_anim: add demuxer fate test for jpegxl_anim
Leo Izen
leo.izen at gmail.com
Wed Jun 28 03:33:16 EEST 2023
On 6/27/23 19:46, James Almer wrote:
> On 6/27/2023 8:32 PM, Leo Izen wrote:
>> On 6/27/23 19:25, James Almer wrote:
>>> On 6/26/2023 12:49 PM, Leo Izen wrote:
>>>> Adds a fate test for the jpegxl_anim demuxer, that should allow testing
>>>> for true positives and false positives for animated jpegxl files. Note
>>>> that two of the test cases are not animated, in order to help sort out
>>>> false positives.
>>>>
>>>> Signed-off-by: <leo.izen at gmail.com>
>>>> ---
>>>> tests/Makefile | 1 +
>>>> tests/fate/jxl.mak | 16 ++++++++++++++++
>>>> tests/ref/fate/jxl-anim-demux-belgium | 6 ++++++
>>>> tests/ref/fate/jxl-anim-demux-icos4d | 6 ++++++
>>>> tests/ref/fate/jxl-anim-demux-lenna256 | 7 +++++++
>>>> tests/ref/fate/jxl-anim-demux-newton | 6 ++++++
>>>> 6 files changed, 42 insertions(+)
>>>> create mode 100644 tests/fate/jxl.mak
>>>> create mode 100644 tests/ref/fate/jxl-anim-demux-belgium
>>>> create mode 100644 tests/ref/fate/jxl-anim-demux-icos4d
>>>> create mode 100644 tests/ref/fate/jxl-anim-demux-lenna256
>>>> create mode 100644 tests/ref/fate/jxl-anim-demux-newton
>>>>
>>>> diff --git a/tests/Makefile b/tests/Makefile
>>>> index e09f30a0fc..7b80762e81 100644
>>>> --- a/tests/Makefile
>>>> +++ b/tests/Makefile
>>>> @@ -201,6 +201,7 @@ include $(SRC_PATH)/tests/fate/image.mak
>>>> include $(SRC_PATH)/tests/fate/imf.mak
>>>> include $(SRC_PATH)/tests/fate/indeo.mak
>>>> include $(SRC_PATH)/tests/fate/jpeg2000.mak
>>>> +include $(SRC_PATH)/tests/fate/jxl.mak
>>>> include $(SRC_PATH)/tests/fate/libavcodec.mak
>>>> include $(SRC_PATH)/tests/fate/libavdevice.mak
>>>> include $(SRC_PATH)/tests/fate/libavformat.mak
>>>> diff --git a/tests/fate/jxl.mak b/tests/fate/jxl.mak
>>>> new file mode 100644
>>>> index 0000000000..057d3be0e1
>>>> --- /dev/null
>>>> +++ b/tests/fate/jxl.mak
>>>> @@ -0,0 +1,16 @@
>>>> +# These two are animated JXL files
>>>> +FATE_JPEGXL_ANIM_DEMUX += fate-jxl-anim-demux-newton
>>>> +fate-jxl-anim-demux-newton: CMD = framecrc -i
>>>> $(TARGET_SAMPLES)/jxl/newton.jxl -c copy
>>>> +FATE_JPEGXL_ANIM_DEMUX += fate-jxl-anim-demux-icos4d
>>>> +fate-jxl-anim-demux-icos4d: CMD = framecrc -i
>>>> $(TARGET_SAMPLES)/jxl/icos4d.jxl -c copy
>>>> +
>>>> +# These two are not animated JXL. They are here to check false
>>>> positives.
>>>> +FATE_JPEGXL_ANIM_DEMUX += fate-jxl-anim-demux-belgium
>>>> +fate-jxl-anim-demux-belgium: CMD = framecrc -i
>>>> $(TARGET_SAMPLES)/jxl/belgium.jxl -c copy
>>>> +FATE_JPEGXL_ANIM_DEMUX += fate-jxl-anim-demux-lenna256
>>>> +fate-jxl-anim-demux-lenna256: CMD = framecrc -i
>>>> $(TARGET_SAMPLES)/jxl/lenna-256.jxl -c copy
>>>> +
>>>> +FATE_JPEGXL_ANIM_DEMUX += $(FATE_JPEGXL_ANIM_DEMUX-yes)
>>>> +
>>>> +FATE_SAMPLES_FFMPEG-$(call FRAMECRC, JPEGXL_ANIM) +=
>>>> $(FATE_JPEGXL_ANIM_DEMUX)
>>>> +fate-jxl-anim-demux: $(FATE_JPEGXL_ANIM_DEMUX)
>>>> diff --git a/tests/ref/fate/jxl-anim-demux-belgium
>>>> b/tests/ref/fate/jxl-anim-demux-belgium
>>>> new file mode 100644
>>>> index 0000000000..b2fe5035ac
>>>> --- /dev/null
>>>> +++ b/tests/ref/fate/jxl-anim-demux-belgium
>>>> @@ -0,0 +1,6 @@
>>>> +#tb 0: 1/25
>>>> +#media_type 0: video
>>>> +#codec_id 0: jpegxl
>>>> +#dimensions 0: 768x512
>>>> +#sar 0: 0/1
>>>> +0, 0, 0, 1, 32, 0xa2930a20
>>>> diff --git a/tests/ref/fate/jxl-anim-demux-icos4d
>>>> b/tests/ref/fate/jxl-anim-demux-icos4d
>>>> new file mode 100644
>>>> index 0000000000..eff6ff1f1b
>>>> --- /dev/null
>>>> +++ b/tests/ref/fate/jxl-anim-demux-icos4d
>>>> @@ -0,0 +1,6 @@
>>>> +#tb 0: 1/1000
>>>> +#media_type 0: video
>>>> +#codec_id 0: jpegxl
>>>> +#dimensions 0: 48x48
>>>> +#sar 0: 0/1
>>>> +0, 0, 0, 0, 67898, 0x53b6516b
>>>> diff --git a/tests/ref/fate/jxl-anim-demux-lenna256
>>>> b/tests/ref/fate/jxl-anim-demux-lenna256
>>>> new file mode 100644
>>>> index 0000000000..0bd286a451
>>>> --- /dev/null
>>>> +++ b/tests/ref/fate/jxl-anim-demux-lenna256
>>>> @@ -0,0 +1,7 @@
>>>> +#tb 0: 1/25
>>>> +#media_type 0: video
>>>> +#codec_id 0: jpegxl
>>>> +#dimensions 0: 256x256
>>>> +#sar 0: 0/1
>>>> +0, 0, 0, 1, 4096, 0x2409e9e3
>>>> +0, 1, 1, 1, 3992, 0x966dbfcb
>>>
>>> What this tells me is that the parser needs to do bitstream assembly
>>> after all. Image2 should not propagate a single image split in two
>>> packets like this, at the arbitrary limit of 4kb.
>>>
>>> Since this format seems to have actual delimiters
>>> (FF_JPEGXL_CODESTREAM_SIGNATURE_LE and
>>> FF_JPEGXL_CONTAINER_SIGNATURE_LE) and even buffer bytes with
>>> ff_jpegxl_collect_codestream_header(), you should then do the
>>> assembly in the parser, much like it's done for bmp, jpg and many
>>> other image formats.
>>> The anim demuxer can remain as PARSE_HEADERS so it doesn't run the
>>> frame assembly code.
>>>
>>
>> The codestream signature is 0xFF 0x0A, which can and frequently will
>> occur unescaped in the middle of a valid file. Detecting the end of
>> the file is also a Hard Problem as it requires an entropy decoder,
>> which I believe you NAK'd for being overly complex for a parser in the
>> first iteration of this a few months ago.
>
> Can you link that first iteration?
https://patchwork.ffmpeg.org/project/ffmpeg/cover/20220401002006.44582-1-leo.izen@gmail.com/
>
>>
>> If I choose to assemble a bitstream here with the parser, what will
>> end up happening is that the entire sequence of all input frames will
>> be one large AVPacket, as detecting the end of the image is
>> nontrivial. Is this behavior desirable, compared to what image2dec
>> does, which is emit arbitrary 4k-sized packets?
>
> What do you mean by all input frames? In the case of animated jpegxl,
> the anim demuxer would handle it, not image2. image2 should be used to
> read and output single frame images.
>
> In any case, what would happen is that image2 will propagate packets of
> up to 4kb size, which the parser invoked in demux.c will then consume
> and not return anything until it has decided it has a complete coded
> image a decoder can then use to fully decode and output a frame, or a
> muxer encapsulate. Said complete coded image is output as a single
> buffer which is propagated to the caller as a single AVPacket.
>
>>
>> If so, I can change it to assemble a packet, but I just want to make
>> sure that behavior is desirable over what I have here.
>>
>> - Leo Izen
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list