On 2/1/22 21:06, James Almer wrote:
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.
The impetus was the desire to use this function in MythTV without
invoking undefined behavior. Currently MythTV uses a modification to
ffmpeg:
https://github.com/MythTV/mythtv/blob/7fa02d4dc3ab0d3f2cbfcbc514047fe2736fe9cf/mythtv/external/FFmpeg/libavcodec/utils-mythtv.c#L228
(For reference, I just tested and a NULL/nullptr const char* becomes an
empty QString, but this is not documented.)
Also, another reason was to match behavior with avcodec_get_name() which
never returns NULL, instead it returns "unknown_codec".
On 2/1/22 21:13, Andreas Rheinhardt wrote:
That would need an announcement in doc/APIchanges; it can then be
changed at the next major version bump after two years have passed.
So, would creating a new function instead and deprecating the old one be
a better option? (e.g. av_get_media_type_name)
Who sets invalid media types?
(In case of fftools/*: If they don't set it themselves, they (and all
our users) should be able to rely on our libraries to not set invalid
values. The same goes for all demuxers (as they set this value
themselves). Checks are only necessary where the media type comes from
the user; this means muxers (where the check should be done generically)
and possibly avfiltergraph.c (I am pretty sure that the type has already
been checked earlier for filters).)
I agree invalid media types are probably not set by anyone, but without
checking for non-NULL it potentially invokes undefined behavior.
Returning "unknown" for AVMEDIA_TYPE_UNKNOWN and returning a different
value for the default case is, in my opinion clearer.
"invalid_media_type" may be a better default string to match the
behavior of avcodec_get_name().
Regards,
Scott
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
