[FFmpeg-devel] [PATCH] lavu: make av_get_media_type_string() never return NULL

[James Almer]

On 2/1/2022 10:58 PM, Scott Theisen wrote:
> On 2/1/22 17:34, James Almer wrote:
>> This is an API breakage, so it's not ok. 
> I'm new, so would this require a bump to the MINOR or MICRO version number?

Making a behavior change like this requires a warning and a period to 
let library users adapt their code. You can't break it just like that 
with no warning at all.

> 
>> The doxy states it returns NULL if media_type is unknown, so you're 
>> expected to check the returned pointer before trying to use it. 
> 
> git grep -E --count "av_get_media_type_string"
> 
> doc/APIchanges:1
> doc/examples/demuxing_decoding.c:5
> doc/examples/extract_mvs.c:2
> fftools/cmdutils.c:1
> fftools/cmdutils.h:1
> fftools/ffmpeg.c:5
> fftools/ffmpeg_opt.c:1
> fftools/ffplay.c:2
> fftools/ffprobe.c:3
> libavcodec/avcodec.c:1
> libavcodec/tests/avcodec.c:1
> libavdevice/dshow.c:2
> libavfilter/avfilter.c:2
> libavfilter/avfiltergraph.c:2
> libavfilter/src_movie.c:3
> libavformat/avienc.c:1
> libavformat/flvenc.c:1
> libavformat/framehash.c:1
> libavformat/mov.c:1
> libavformat/mpegenc.c:1
> libavformat/mpegts.c:1
> libavformat/mxfdec.c:1
> libavformat/segment.c:1
> libavformat/tee.c:1
> libavformat/uncodedframecrcenc.c:1
> libavutil/avutil.h:1
> libavutil/utils.c:1
> 
> 44 total - 1 non-code - 1 prototype - 1 definition + 4 via macro - 1 
> macro = 44 uses
> 
> The following uses of av_get_media_type_string() do not check for null:
> doc/examples/demuxing_decoding.c: 5
> doc/examples/extract_mvs.c: 2
> fftools/cmdutils.c: 2 via the define in cmdutils.h
> fftools/ffmpeg.c: 7, 2 via the define in cmdutils.h
> fftools/ffmpeg_opt.c:1
> fftools/ffplay.c:2
> libavfilter/avfiltergraph.c:2
> libavfilter/src_movie.c:3
> libavformat/flvenc.c:1
> libavformat/framehash.c:1
> libavformat/mov.c:1
> libavformat/mpegenc.c:1
> libavformat/mpegts.c:1
> libavformat/mxfdec.c:1
> libavformat/segment.c:1
> libavformat/tee.c:1
> 
> 32/44 uses do not check for null.

But do they need to? Those modules have probably ensured media type is 
one of the known and supported ones by the time they call this function.

Why do you need this function to never return NULL, anyway? If you know 
any of these calls where media type can be UNKNOWN, then it's easier to 
just fix them by checking for NULL as per the doxy.

> 
> 
> libavcodec/tests/avcodec.c: this use is a workaround to 
> av_get_media_type_string() returning null; checked 3 times
> 
> 
> Let me know which version number to bump and I will submit a new version 
> that also removes the now redundant null checks from the rest of the code.
> 
> Thanks,
> Scott
> _______________________________________________
> 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".