Re: [FFmpeg-devel] [PATCH v3 3/4] avformat/jpegxl: remove jpegxl_probe, instead call avcodec/jpegxl_parse

From: James Almer <jamrial@gmail.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH v3 3/4] avformat/jpegxl: remove jpegxl_probe, instead call avcodec/jpegxl_parse
Date: Sat, 24 Jun 2023 09:21:37 -0300
Message-ID: <ca226722-4edb-8cb2-56ea-4e36b53592d7@gmail.com> (raw)
In-Reply-To: <20230624043853.12237-4-leo.izen@gmail.com>

On 6/24/2023 1:38 AM, Leo Izen wrote:
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 2b90b788c0..0637199c9d 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -283,7 +283,7 @@ OBJS-$(CONFIG_IMAGE_HDR_PIPE_DEMUXER)     += img2dec.o img2.o
>   OBJS-$(CONFIG_IMAGE_J2K_PIPE_DEMUXER)     += img2dec.o img2.o
>   OBJS-$(CONFIG_IMAGE_JPEG_PIPE_DEMUXER)    += img2dec.o img2.o
>   OBJS-$(CONFIG_IMAGE_JPEGLS_PIPE_DEMUXER)  += img2dec.o img2.o
> -OBJS-$(CONFIG_IMAGE_JPEGXL_PIPE_DEMUXER)  += img2dec.o img2.o jpegxl_probe.o
> +OBJS-$(CONFIG_IMAGE_JPEGXL_PIPE_DEMUXER)  += img2dec.o img2.o jpegxl_parse.o
>   OBJS-$(CONFIG_IMAGE_PAM_PIPE_DEMUXER)     += img2dec.o img2.o
>   OBJS-$(CONFIG_IMAGE_PBM_PIPE_DEMUXER)     += img2dec.o img2.o
>   OBJS-$(CONFIG_IMAGE_PCX_PIPE_DEMUXER)     += img2dec.o img2.o
> @@ -320,7 +320,7 @@ OBJS-$(CONFIG_IVF_MUXER)                 += ivfenc.o
>   OBJS-$(CONFIG_IVR_DEMUXER)               += rmdec.o rm.o rmsipr.o
>   OBJS-$(CONFIG_JACOSUB_DEMUXER)           += jacosubdec.o subtitles.o
>   OBJS-$(CONFIG_JACOSUB_MUXER)             += jacosubenc.o rawenc.o
> -OBJS-$(CONFIG_JPEGXL_ANIM_DEMUXER)       += jpegxl_anim_dec.o jpegxl_probe.o
> +OBJS-$(CONFIG_JPEGXL_ANIM_DEMUXER)       += jpegxl_anim_dec.o jpegxl_parse.o

These two will compile jpegxl_parse.o twice on static builds, once in 
lavc and once in lavf, which is a waste. You instead need to add

STLIBOBJS-$(CONFIG_IMAGE_JPEGXL_PIPE_DEMUXER) += jpegxl_parse.o
STLIBOBJS-$(CONFIG_JPEGXL_ANIM_DEMUXER) += jpegxl_parse.o

to libavcodec/Makefile

>   OBJS-$(CONFIG_JV_DEMUXER)                += jvdec.o
>   OBJS-$(CONFIG_KUX_DEMUXER)               += flvdec.o
>   OBJS-$(CONFIG_KVAG_DEMUXER)              += kvag.o
> @@ -715,6 +715,7 @@ SHLIBOBJS                                += log2_tab.o to_upper4.o
>   SHLIBOBJS-$(CONFIG_ISO_MEDIA)            += mpegaudiotabs.o
>   SHLIBOBJS-$(CONFIG_FLV_MUXER)            += mpeg4audio_sample_rates.o
>   SHLIBOBJS-$(CONFIG_HLS_DEMUXER)          += ac3_channel_layout_tab.o
> +SHLIBOBJS-$(CONFIG_JPEGXL_ANIM_DEMUXER)  += jpegxl_parse.o

Missing the pipe demuxer here too.

>   SHLIBOBJS-$(CONFIG_MATROSKA_DEMUXER)     += mpeg4audio_sample_rates.o
>   SHLIBOBJS-$(CONFIG_MOV_DEMUXER)          += ac3_channel_layout_tab.o
>   SHLIBOBJS-$(CONFIG_MP3_MUXER)            += mpegaudiotabs.o
> diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
> index b986d3a502..72b2c76405 100644
> --- a/libavformat/img2dec.c
> +++ b/libavformat/img2dec.c
> @@ -36,7 +36,7 @@
>  #include "avio_internal.h"
>  #include "internal.h"
>  #include "img2.h"
> -#include "jpegxl_probe.h"
> +#include "libavcodec/jpegxl_parse.h"
>  #include "libavcodec/mjpeg.h"
>  #include "libavcodec/vbn.h"
>  #include "libavcodec/xwd.h"
> @@ -850,7 +850,7 @@ static int jpegxl_probe(const AVProbeData *p)
>      if (AV_RL16(b) != FF_JPEGXL_CODESTREAM_SIGNATURE_LE)
>          return 0;
>  #if CONFIG_IMAGE_JPEGXL_PIPE_DEMUXER
> -    if (ff_jpegxl_verify_codestream_header(p->buf, p->buf_size, 1) >= 0)
> +    if (ff_jpegxl_parse_codestream_header(p->buf, p->buf_size, 1, NULL) >= 0)

If it's only for the probe function, can't you add a reduced version 
here that can also conclude the stream is jpegxl? That way you don't 
need it to depend on lavc code and simplifies the stuff above.

>          return AVPROBE_SCORE_MAX - 2;

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".