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

[Leo Izen <leo.izen@gmail.com>]

On 6/24/23 08:21, James Almer wrote:
> 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


Will fix.

> 
>>   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.


Will fix.

> 
>>   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.
> 

I tried repeatedly to do this, and it kept failing the probetest. I 
don't believe there's a simplified way to do it that won't fail that.

- Leo Izen
_______________________________________________
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".