On 7/20/2020 9:35 PM, Andreas Rheinhardt wrote: > James Almer: >> On 7/19/2020 5:47 PM, Andreas Rheinhardt wrote: >>> If reading the header fails, the demuxer's read_close() function (if >>> existing) is not called automatically; instead several demuxers call it >>> via "goto fail" in read_header(). >>> >>> This commit intends to change this by adding a flag to AVInputFormat >>> that can be used to set on a per-AVInputFormat basis whether read_close() >>> should be called generically after an error during read_header(). >>> >>> The flag controlling this behaviour needs to be added because it might >>> be unsafe to call read_close() generally (e.g. this might lead to >>> read_close() being called twice and this might e.g. lead to double-frees >>> if av_free() is used instead of av_freep(); or a size field has not >>> been reset after freeing the elements (see the mov demuxer for an >>> example of this)). Yet the intention is to check and fix all demuxers >>> and make the flag redundant in the medium run. >>> >>> The flag itself is non-public (it resides in libavformat/internal.h), >>> but it has been added to the ordinary (i.e. public) flags field of >>> AVInputFormat, because there is no field for internal flags and adding >>> one is not possible, because libavdevice also defines AVInputFormats >>> and so there is the possibility that a newer libavformat is used >>> together with an older libavdevice that would then lack the new field >>> for internal flags. When it has become redundant, it can be removed again >>> at the next major version bump. >>> >>> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> >>> --- >>> This is an updated version of my initial patch [1]. I have also rebased >>> the whole set of patches following it (with the exception of the w3c >>> patch in the next patch they no longer fix a memleak; instead they now >>> only set the flag and remove the manual calls to read_close). Should I >>> resend the other patches, too? >>> >>> [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-March/258830.html >>> >>> libavformat/internal.h | 6 ++++++ >>> libavformat/utils.c | 11 +++++++++-- >>> 2 files changed, 15 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavformat/internal.h b/libavformat/internal.h >>> index 17a6ab07d3..b7441a5959 100644 >>> --- a/libavformat/internal.h >>> +++ b/libavformat/internal.h >>> @@ -39,6 +39,12 @@ >>> # define hex_dump_debug(class, buf, size) do { if (0) >>> av_hex_dump_log(class, AV_LOG_DEBUG, buf, size); } while(0) >>> #endif >>> >>> +/** Internal flag that is part of AVInputFormat.flags due to >>> + * ABI restrictions that forbid adding a new flags_internal >>> + * to AVInputFormat. */ >> >> You can add fields below the "Not public" notice with a minor bump. >> Nothing is meant to access those directly, except unfortunately lavd. >> And if you're effectively talking about lavd, then adding it at the end >> should not affect the offsets of fields currently accessed by indevs. Or >> am i missing something? >> > The point is that it is (unfortunately) allowed to use an older > libavdevice together with a newer libavformat. This means it is possible > that the AVInputFormat used in avformat_open_input() may be from a > libavdevice that is so old that it doesn't have the new internal flags > field yet. So one either uses an unused bit of the ordinary flags or one > adds functions that allow to check whether a given AVInputFormat is an > input device from a too old libavdevice. Or one waits with this change > until the next major bump and does everything at once.
Wouldn't a check for AV_IS_INPUT_DEVICE(ifmt->priv_class->category) before trying to look at the new flags_internal field in avformat_open_input() be enough? Perhaps with an avdevice_version() check as well in case a device cares about using it. This can be safely removed after a major bump. > > Notice that in any case, every demuxer with a read_close function needs > to be checked for compatibility with calling read_close generically on > read_header error. I have just found two more (besides mov) demuxers > (rmdec and concat) that are not. Adding this flag allows to easily see > which demuxers have already been checked. > > - Andreas > _______________________________________________ > 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". > _______________________________________________ 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".
