[FFmpeg-devel] [PATCH 08/13] avformat/matroskaenc: Remove inconsistencies wrt seekability handling
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Sat May 2 20:16:55 EEST 2020
The Matroska muxer behaves differently in several ways when it thinks
that it is in unseekable/livestreaming mode: It does not add Cue entries
because they won't be written anyway for a livestream and it writes some
elements only preliminarily (with the intention to overwrite them with
an updated version at the end) when non-livestreaming etc.
There are two ways to set the Matroska muxer into livestreaming mode:
Setting an option or by providing an unseekable AVIOContext. Yet the
actual checks were not consistent:
If the AVIOContext was unseekable and no AAC extradata was available
when writing the header, writing the header failed; but if the AVIOContext
was seekable, it didn't, because the muxer expected to get the extradata
via packet side-data. Here the livestreaming option has not been checked,
although one can't use the updated extradata in case it is a livestream.
If the reserve_index_space option was used, space for writing Cues would
be reserved when writing the header unless the AVIOContext was
unseekable. Yet Cues were only written if the livestreaming option was
not set and the AVIOContext was seekable (when writing the trailer), so
if the AVIOContext was seekable and the livestreaming option set, the
reserved space would never be used at all.
If the AVIOContext was unseekable and the livestreaming option was not
set, it would be attempted to update the main length field at the end.
After all, it might be possible that the file is so short that it fits
into the AVIOContext's buffer in which case the seek back would work.
Yet this is dangerous: It might be that we are not dealing with a
simple output file, but that our output gets split into chunks and that
each of these chunks is actually seekable. In this case some part of the
last chunk (namely the eight bytes that have the same offset as the
length field had in the header) will be overwritten with what the muxer
wrongly believes to be the filesize.
(The livestreaming option has been added to deal with this scenario,
yet its documentation ("Write files assuming it is a live stream.")
doesn't make this clear at all. At least the segment muxer does not
set the option for live and given that the chances of successfully
seeking when the output is actually unseekable are slim, it is best to
not attempt to update the length field in the unseekable case at all.)
All these inconsistencies were fixed by treating the output as seekable
if the livestreaming option is not set and if the AVIOContext is
seekable. A macro has been used to enforce consistency and improve code
readability.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
I am actually not really satisfied with this: I'd really like an option
that says that the output is a single file. The reason for this is
that I am working on a patchset for writing attached picture streams as
AttachedFile in Matroska (there are several tickets open for this), yet
the corresponding packets are only available after writing the header.*
My plan is to write the Attachments as soon as possible when seekable
(i.e. before the first Cluster when all necessary packets already arrived;
at the end otherwise) and if I knew that the output was a single file
(even if unseekable) I could also write Attachments before the first
Cluster.
This commit will btw lead to a misleading-indentation warning. It will
be fixed in the cosmetics commit.
*: I have an idea to mitigate this (by using AVStream.attached_pic for
muxers, too), but it won't solve this problem completely.
libavformat/matroskaenc.c | 37 ++++++++++++++++++++-----------------
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 1a142403fa..ccbd73cbe4 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -58,6 +58,9 @@
* Info, Tracks, Chapters, Attachments, Tags and Cues */
#define MAX_SEEKHEAD_ENTRIES 6
+#define IS_SEEKABLE(pb, mkv) (((pb)->seekable & AVIO_SEEKABLE_NORMAL) && \
+ !(mkv)->is_live)
+
enum {
DEFAULT_MODE_INFER,
DEFAULT_MODE_INFER_NO_SUBS,
@@ -671,9 +674,9 @@ static int put_flac_codecpriv(AVFormatContext *s, AVIOContext *pb,
return 0;
}
-static int get_aac_sample_rates(AVFormatContext *s, const uint8_t *extradata,
- int extradata_size, int *sample_rate,
- int *output_sample_rate)
+static int get_aac_sample_rates(AVFormatContext *s, MatroskaMuxContext *mkv,
+ const uint8_t *extradata, int extradata_size,
+ int *sample_rate, int *output_sample_rate)
{
MPEG4AudioConfig mp4ac;
int ret;
@@ -684,7 +687,7 @@ static int get_aac_sample_rates(AVFormatContext *s, const uint8_t *extradata,
* first packet.
* Abort however if s->pb is not seekable, as we would not be able to seek back
* to write the sample rate elements once the extradata shows up, anyway. */
- if (ret < 0 && (extradata_size || !(s->pb->seekable & AVIO_SEEKABLE_NORMAL))) {
+ if (ret < 0 && (extradata_size || !IS_SEEKABLE(s->pb, mkv))) {
av_log(s, AV_LOG_ERROR,
"Error parsing AAC extradata, unable to determine samplerate.\n");
return AVERROR(EINVAL);
@@ -1141,8 +1144,8 @@ static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
return 0;
if (par->codec_id == AV_CODEC_ID_AAC) {
- ret = get_aac_sample_rates(s, par->extradata, par->extradata_size, &sample_rate,
- &output_sample_rate);
+ ret = get_aac_sample_rates(s, mkv, par->extradata, par->extradata_size,
+ &sample_rate, &output_sample_rate);
if (ret < 0)
return ret;
}
@@ -1907,11 +1910,13 @@ static int mkv_write_header(AVFormatContext *s)
put_ebml_void(pb, s->metadata_header_padding);
}
- if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && mkv->reserve_cues_space) {
+ if (mkv->reserve_cues_space) {
+ if (IS_SEEKABLE(pb, mkv)) {
mkv->cues_pos = avio_tell(pb);
if (mkv->reserve_cues_space == 1)
mkv->reserve_cues_space++;
put_ebml_void(pb, mkv->reserve_cues_space);
+ }
}
av_init_packet(&mkv->cur_audio_pkt);
@@ -1920,7 +1925,7 @@ static int mkv_write_header(AVFormatContext *s)
// start a new cluster every 5 MB or 5 sec, or 32k / 1 sec for streaming or
// after 4k and on a keyframe
- if (pb->seekable & AVIO_SEEKABLE_NORMAL) {
+ if (IS_SEEKABLE(pb, mkv)) {
if (mkv->cluster_time_limit < 0)
mkv->cluster_time_limit = 5000;
if (mkv->cluster_size_limit < 0)
@@ -2188,8 +2193,8 @@ static int mkv_check_new_extra_data(AVFormatContext *s, const AVPacket *pkt)
case AV_CODEC_ID_AAC:
if (side_data_size && mkv->track.bc) {
int filler, output_sample_rate = 0;
- ret = get_aac_sample_rates(s, side_data, side_data_size, &track->sample_rate,
- &output_sample_rate);
+ ret = get_aac_sample_rates(s, mkv, side_data, side_data_size,
+ &track->sample_rate, &output_sample_rate);
if (ret < 0)
return ret;
if (!output_sample_rate)
@@ -2311,7 +2316,7 @@ static int mkv_write_packet_internal(AVFormatContext *s, const AVPacket *pkt)
ret = mkv_write_block(s, pb, MATROSKA_ID_SIMPLEBLOCK, pkt, keyframe);
if (ret < 0)
return ret;
- if ((s->pb->seekable & AVIO_SEEKABLE_NORMAL) && keyframe &&
+ if (keyframe && IS_SEEKABLE(s->pb, mkv) &&
(par->codec_type == AVMEDIA_TYPE_VIDEO || !mkv->have_video && !track->has_cue)) {
ret = mkv_add_cuepoint(mkv, pkt->stream_index, ts,
mkv->cluster_pos, relative_packet_pos, -1);
@@ -2341,7 +2346,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
end_ebml_master(pb, blockgroup);
}
- if (s->pb->seekable & AVIO_SEEKABLE_NORMAL) {
+ if (IS_SEEKABLE(s->pb, mkv)) {
ret = mkv_add_cuepoint(mkv, pkt->stream_index, ts,
mkv->cluster_pos, relative_packet_pos, duration);
if (ret < 0)
@@ -2451,6 +2456,7 @@ static int mkv_write_trailer(AVFormatContext *s)
{
MatroskaMuxContext *mkv = s->priv_data;
AVIOContext *pb = s->pb;
+ int64_t endpos, ret64;
int ret;
// check if we have an audio packet cached
@@ -2474,9 +2480,8 @@ static int mkv_write_trailer(AVFormatContext *s)
if (ret < 0)
return ret;
-
- if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live) {
- int64_t endpos, ret64;
+ if (!IS_SEEKABLE(pb, mkv))
+ return 0;
endpos = avio_tell(pb);
@@ -2592,9 +2597,7 @@ static int mkv_write_trailer(AVFormatContext *s)
}
avio_seek(pb, endpos, SEEK_SET);
- }
- if (!mkv->is_live)
end_ebml_master(pb, mkv->segment);
return mkv->reserve_cues_space < 0 ? AVERROR(EINVAL) : 0;
--
2.20.1
More information about the ffmpeg-devel
mailing list