On Sun, 6 Dec 2020, Andreas Rheinhardt wrote:
Marton Balint:
On Sat, 5 Dec 2020, Andreas Rheinhardt wrote:
Normally, video packets are muxed before audio packets for mxf (there is
a dedicated interleave function for this); furthermore the first (video)
packet triggers writing the actual header. Yet when the first video
packet
fails the checks performed on it, codec_ul (a value set based upon
properties of the bitstream which necessitates actually inspecting
packets) may be NULL; the next packet written (an audio packet) will then
trigger outputting the header and this leads to a segfault because
codec_ul is dereferenced (this is ticket #7993). Therefore this commit
discards audio packets until a valid video packet has been received,
fixing said ticket.
I don't think in general it is expected by muxers to do anything useful
if a write_packet call fails and a subsequent write_packet call is
attempted.
A write_packet call error is not an error a muxer can recover from, and
attempting another write_packet seems like undefined behaviour to me,
admittedly this is not really documented.
While this is true in lots of error scenarios, it is not true for all of
them. It can also be that it is just a single packet that is crap and
gets rejected; e.g. the Matroska muxer rejects packets without PTS (i.e.
pkt->pts == AV_NO_PTS_VALUE), yet packets after that can be handled just
fine.
This looks like something that happens to work this way, not
intentionally. And how broken the file will be for which the muxer
rejected some packets?
Another case are some output devices that return AVERROR(EAGAIN).
Good point, EAGAIN should not be considered fatal. Although EAGAIN is kind
of an exception, because that means that you should wait some time and
retry the same packet later, and not continue with the next packet...
And given that no prohibition is documented, adding such a prohibition
would be an API break that could would require a deprecation period.
I think you overestimate the usefulness of this feature. Sure, we can
wait the API bump with this change, but I don't think it's worth waiting
for a deprecation period.
Anyhow, since general solution clearly won't happen anytime soon, feel
free to apply the patch as is.
But I am certainly not against a more generic solution. How about a new
field in AVFormatInternal that a muxer can set if writing more packets
is hopeless and should not be attempted any more and if it is set and a
new attempt at writing a packet is attempted, it gets rejected without
reaching the muxer at all.
This should be opt-out, not opt-in IMHO, because not counting EAGAIN I
am still skeptical about a particularly useful scenario. Maybe a muxer
flag is enough? Although before adding any support for this, I still would
like to see a justified use.
In the ticket above, it is write_trailer that is crashing because
write_trailer tries to output more packets after an earlier write_packet
failure.
Sure about that? My stacktrace says that it comes from
av_interleaved_write_frame(); av_write_trailer() is never called.
I tested the command line on the ticket:
ffmpeg_g -y -i tmp.vob -map 0 -c:v prores_ks -c:v:122 fits
-disposition:a:39 h261 -disposition:a:114 wmv1 -vframes 17 -b:v 587
-strict 1 tmp_.mxf
and got this with gdb at the crash:
__memcpy_ssse3 at /lib64/libc.so.6
avio_write at libavformat/aviobuf.c:234
mxf_write_cdci_common at libavformat/mxfenc.c:1244
mxf_write_cdci_desc at libavformat/mxfenc.c:1324
mxf_write_package at libavformat/mxfenc.c:1612
mxf_write_header_metadata_sets at libavformat/mxfenc.c:1683
mxf_write_partition at libavformat/mxfenc.c:1944
mxf_write_packet at libavformat/mxfenc.c:2903
write_packet at libavformat/mux.c:731
interleaved_write_packet at libavformat/mux.c:1104
av_write_trailer at libavformat/mux.c:1266
transcode at fftools/ffmpeg.c:4795
main at fftools/ffmpeg.c:4966
Regards,
Marton
_______________________________________________
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".
