mbox series

[FFmpeg-devel,v7,0/5] JPEG XL Parser (and bug fixes)

Message ID 20230802203334.23640-1-leo.izen@gmail.com
Headers show
Series JPEG XL Parser (and bug fixes) | expand

Message

Leo Izen Aug. 2, 2023, 8:33 p.m. UTC
Changes from v6:
- Added dummy stub libavformat/jpegxl_parse.c to fix shared builds

Changes from v5:
- Attached an extra commit to fix existing bug with libjxldec
- Made various changes based on comments by Andreas Reinhardt
-- removed jpegxl_parse.c from avformat, and jpegxl_parse.o from avformat/Makefile/OBJS
-- checked for failure in init_vlc_lengths
-- used a macro to initialize VLCElem structs
-- used heap allocations for a buffer of size 250k, instead of stack
-- renamed "code" to "len"
- Fix demuxer to avoid using avio_size, breaking piped input
- Fix some parser bugs discovered during more extensive testing

Changes from v4:
- Added an entropy decoder and full parser, which finds the
    boundaries between files correctly
- Removed unnecessary logging in libjxldec

Changes from v3:

- Don't remove AV_CODEC_CAP_DR1 from libjxldec
- jpegxl_parse.o added to STLIBOBJS in avcodec/Makefile
- add pipe demuxer to avformat/Makefile's SHLIBOBJS

Changes from v2:

- Fix libjxldec to work with packets that are smaller than one frame
- Change how code is shared between libavcodec and libavformat to be more sensible.
- Fix the parser to work with large headers that proceed the codestream in a container format
    (for example, if several-KB Exif boxes preceed the codestream.)
- Modify the parser to set width/height instead of avctx
  Note: avctx->pix_fmt and s->format are both set, because otherwise the CLI tools won't print
    the pixel format without libjxl enabled.
- Update the fate test based on the new parser's packetization

This test relies on FATE samples that haven't been uploaded yet. To test, unzip
the following zipfile[1] in the FATE_SAMPLES directory, placing the test images in jxl/.

[1]: https://buzo.us/y.zip

sha256sum: 43a2eeb0dfdf471b47a9fdfb1653974fa156ceceb776891cc137569a8ebf0e75
signature: https://buzo.us/R.asc

Leo Izen (5):
  avcodec/libjxldec: fix errors when decoding grayscale after rgb
  avcodec/libjxldec: use internal AVFrame as buffered space
  avcodec/jpegxl_parser: add JPEG XL parser
  avformat/jpegxl: remove jpegxl_probe, instead call
    avcodec/jpegxl_parse
  fate/jpegxl_anim: add demuxer fate test for jpegxl_anim

 libavcodec/Makefile                           |    3 +
 libavcodec/jpegxl.h                           |   94 ++
 libavcodec/jpegxl_parse.c                     |  520 ++++++
 libavcodec/jpegxl_parse.h                     |   72 +
 libavcodec/jpegxl_parser.c                    | 1477 +++++++++++++++++
 libavcodec/libjxldec.c                        |   41 +-
 libavcodec/parsers.c                          |    1 +
 libavcodec/version.h                          |    2 +-
 libavformat/Makefile                          |    6 +-
 libavformat/img2dec.c                         |    4 +-
 libavformat/jpegxl_anim_dec.c                 |  132 +-
 .../{jpegxl_probe.h => jpegxl_parse.c}        |   21 +-
 libavformat/jpegxl_probe.c                    |  412 -----
 libavformat/version.h                         |    2 +-
 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        |    6 +
 tests/ref/fate/jxl-anim-demux-newton          |    6 +
 20 files changed, 2273 insertions(+), 555 deletions(-)
 create mode 100644 libavcodec/jpegxl.h
 create mode 100644 libavcodec/jpegxl_parse.c
 create mode 100644 libavcodec/jpegxl_parse.h
 create mode 100644 libavcodec/jpegxl_parser.c
 rename libavformat/{jpegxl_probe.h => jpegxl_parse.c} (55%)
 delete mode 100644 libavformat/jpegxl_probe.c
 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

Comments

Leo Izen Aug. 9, 2023, 9:56 p.m. UTC | #1
On 8/2/23 16:33, Leo Izen wrote:
> Changes from v6:
> - Added dummy stub libavformat/jpegxl_parse.c to fix shared builds

Bumping for review. I know the commit message in patch 3/5 is wrong, but 
I can fix that on merge, or v8 if a v8 is necessary.

- Leo Izen
Leo Izen Aug. 16, 2023, 4:25 a.m. UTC | #2
On 8/9/23 17:56, Leo Izen wrote:
> On 8/2/23 16:33, Leo Izen wrote:
>> Changes from v6:
>> - Added dummy stub libavformat/jpegxl_parse.c to fix shared builds
> 
> Bumping for review. I know the commit message in patch 3/5 is wrong, but 
> I can fix that on merge, or v8 if a v8 is necessary.
> 
> - Leo Izen
> 

Bumping again. This is a lot of code, so I don't want to threaten to 
merge it, so if you take a look and having nothing more to add then a 
LGTM would be appreciated, thanks.

- Leo Izen
Leo Izen Aug. 24, 2023, 7:22 p.m. UTC | #3
On 8/2/23 16:33, Leo Izen wrote:
> Changes from v6:
> - Added dummy stub libavformat/jpegxl_parse.c to fix shared builds
> 
> Changes from v5:
> - Attached an extra commit to fix existing bug with libjxldec
> - Made various changes based on comments by Andreas Reinhardt
> -- removed jpegxl_parse.c from avformat, and jpegxl_parse.o from avformat/Makefile/OBJS
> -- checked for failure in init_vlc_lengths
> -- used a macro to initialize VLCElem structs
> -- used heap allocations for a buffer of size 250k, instead of stack
> -- renamed "code" to "len"
> - Fix demuxer to avoid using avio_size, breaking piped input
> - Fix some parser bugs discovered during more extensive testing
> 
> Changes from v4:
> - Added an entropy decoder and full parser, which finds the
>      boundaries between files correctly
> - Removed unnecessary logging in libjxldec
> 
> Changes from v3:
> 
> - Don't remove AV_CODEC_CAP_DR1 from libjxldec
> - jpegxl_parse.o added to STLIBOBJS in avcodec/Makefile
> - add pipe demuxer to avformat/Makefile's SHLIBOBJS
> 
> Changes from v2:
> 
> - Fix libjxldec to work with packets that are smaller than one frame
> - Change how code is shared between libavcodec and libavformat to be more sensible.
> - Fix the parser to work with large headers that proceed the codestream in a container format
>      (for example, if several-KB Exif boxes preceed the codestream.)
> - Modify the parser to set width/height instead of avctx
>    Note: avctx->pix_fmt and s->format are both set, because otherwise the CLI tools won't print
>      the pixel format without libjxl enabled.
> - Update the fate test based on the new parser's packetization
> 
> This test relies on FATE samples that haven't been uploaded yet. To test, unzip
> the following zipfile[1] in the FATE_SAMPLES directory, placing the test images in jxl/.
> 
> [1]: https://buzo.us/y.zip
> 
> sha256sum: 43a2eeb0dfdf471b47a9fdfb1653974fa156ceceb776891cc137569a8ebf0e75
> signature: https://buzo.us/R.asc
> 
> Leo Izen (5):
>    avcodec/libjxldec: fix errors when decoding grayscale after rgb
>    avcodec/libjxldec: use internal AVFrame as buffered space
>    avcodec/jpegxl_parser: add JPEG XL parser
>    avformat/jpegxl: remove jpegxl_probe, instead call
>      avcodec/jpegxl_parse
>    fate/jpegxl_anim: add demuxer fate test for jpegxl_anim
> 
>   libavcodec/Makefile                           |    3 +
>   libavcodec/jpegxl.h                           |   94 ++
>   libavcodec/jpegxl_parse.c                     |  520 ++++++
>   libavcodec/jpegxl_parse.h                     |   72 +
>   libavcodec/jpegxl_parser.c                    | 1477 +++++++++++++++++
>   libavcodec/libjxldec.c                        |   41 +-
>   libavcodec/parsers.c                          |    1 +
>   libavcodec/version.h                          |    2 +-
>   libavformat/Makefile                          |    6 +-
>   libavformat/img2dec.c                         |    4 +-
>   libavformat/jpegxl_anim_dec.c                 |  132 +-
>   .../{jpegxl_probe.h => jpegxl_parse.c}        |   21 +-
>   libavformat/jpegxl_probe.c                    |  412 -----
>   libavformat/version.h                         |    2 +-
>   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        |    6 +
>   tests/ref/fate/jxl-anim-demux-newton          |    6 +
>   20 files changed, 2273 insertions(+), 555 deletions(-)
>   create mode 100644 libavcodec/jpegxl.h
>   create mode 100644 libavcodec/jpegxl_parse.c
>   create mode 100644 libavcodec/jpegxl_parse.h
>   create mode 100644 libavcodec/jpegxl_parser.c
>   rename libavformat/{jpegxl_probe.h => jpegxl_parse.c} (55%)
>   delete mode 100644 libavformat/jpegxl_probe.c
>   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
> 

Merging tomorrow as this has been through several review cycles.

- Leo Izen
Leo Izen Aug. 27, 2023, 5:40 a.m. UTC | #4
On 8/24/23 15:22, Leo Izen wrote:
> On 8/2/23 16:33, Leo Izen wrote:
>> Changes from v6:
>> - Added dummy stub libavformat/jpegxl_parse.c to fix shared builds
>>
>> Changes from v5:
>> - Attached an extra commit to fix existing bug with libjxldec
>> - Made various changes based on comments by Andreas Reinhardt
>> -- removed jpegxl_parse.c from avformat, and jpegxl_parse.o from 
>> avformat/Makefile/OBJS
>> -- checked for failure in init_vlc_lengths
>> -- used a macro to initialize VLCElem structs
>> -- used heap allocations for a buffer of size 250k, instead of stack
>> -- renamed "code" to "len"
>> - Fix demuxer to avoid using avio_size, breaking piped input
>> - Fix some parser bugs discovered during more extensive testing
>>
>> Changes from v4:
>> - Added an entropy decoder and full parser, which finds the
>>      boundaries between files correctly
>> - Removed unnecessary logging in libjxldec
>>
>> Changes from v3:
>>
>> - Don't remove AV_CODEC_CAP_DR1 from libjxldec
>> - jpegxl_parse.o added to STLIBOBJS in avcodec/Makefile
>> - add pipe demuxer to avformat/Makefile's SHLIBOBJS
>>
>> Changes from v2:
>>
>> - Fix libjxldec to work with packets that are smaller than one frame
>> - Change how code is shared between libavcodec and libavformat to be 
>> more sensible.
>> - Fix the parser to work with large headers that proceed the 
>> codestream in a container format
>>      (for example, if several-KB Exif boxes preceed the codestream.)
>> - Modify the parser to set width/height instead of avctx
>>    Note: avctx->pix_fmt and s->format are both set, because otherwise 
>> the CLI tools won't print
>>      the pixel format without libjxl enabled.
>> - Update the fate test based on the new parser's packetization
>>
>> This test relies on FATE samples that haven't been uploaded yet. To 
>> test, unzip
>> the following zipfile[1] in the FATE_SAMPLES directory, placing the 
>> test images in jxl/.
>>
>> [1]: https://buzo.us/y.zip
>>
>> sha256sum: 
>> 43a2eeb0dfdf471b47a9fdfb1653974fa156ceceb776891cc137569a8ebf0e75
>> signature: https://buzo.us/R.asc
>>
>> Leo Izen (5):
>>    avcodec/libjxldec: fix errors when decoding grayscale after rgb
>>    avcodec/libjxldec: use internal AVFrame as buffered space
>>    avcodec/jpegxl_parser: add JPEG XL parser
>>    avformat/jpegxl: remove jpegxl_probe, instead call
>>      avcodec/jpegxl_parse
>>    fate/jpegxl_anim: add demuxer fate test for jpegxl_anim
>>
>>   libavcodec/Makefile                           |    3 +
>>   libavcodec/jpegxl.h                           |   94 ++
>>   libavcodec/jpegxl_parse.c                     |  520 ++++++
>>   libavcodec/jpegxl_parse.h                     |   72 +
>>   libavcodec/jpegxl_parser.c                    | 1477 +++++++++++++++++
>>   libavcodec/libjxldec.c                        |   41 +-
>>   libavcodec/parsers.c                          |    1 +
>>   libavcodec/version.h                          |    2 +-
>>   libavformat/Makefile                          |    6 +-
>>   libavformat/img2dec.c                         |    4 +-
>>   libavformat/jpegxl_anim_dec.c                 |  132 +-
>>   .../{jpegxl_probe.h => jpegxl_parse.c}        |   21 +-
>>   libavformat/jpegxl_probe.c                    |  412 -----
>>   libavformat/version.h                         |    2 +-
>>   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        |    6 +
>>   tests/ref/fate/jxl-anim-demux-newton          |    6 +
>>   20 files changed, 2273 insertions(+), 555 deletions(-)
>>   create mode 100644 libavcodec/jpegxl.h
>>   create mode 100644 libavcodec/jpegxl_parse.c
>>   create mode 100644 libavcodec/jpegxl_parse.h
>>   create mode 100644 libavcodec/jpegxl_parser.c
>>   rename libavformat/{jpegxl_probe.h => jpegxl_parse.c} (55%)
>>   delete mode 100644 libavformat/jpegxl_probe.c
>>   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
>>
> 
> Merging tomorrow as this has been through several review cycles.
> 
> - Leo Izen

Pushed all except the last patch as the samples haven't been uploaded 
yet, so FATE will fail if I do that.

- Leo Izen (Traneptora)