avformat/matroskaenc: Remove inconsistencies wrt seekability handling

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@gmail.com>

1 files 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;