[FFmpeg-devel] [PATCH 15/23] avformat/matroskaenc: Don't use size of inexistent cluster

[Andreas Rheinhardt]

In order to determine whether the current cluster needs to be closed
because of the limits on cluster-size and cluster-time,
mkv_write_packet() would first get the size of the current cluster via
applying avio_tell() on the dynamic buffer holding the current cluster.
It did this without checking whether there is an open cluster right now,
so that avio_tell() returned AVERROR(EINVAL) which worked as expected
because the return value was put into a signed type (the call was
unchecked).

This is not good as it relied on avio_tell() (or actually, avio_seek())
to handle the case of an inexistent AVIOContext gracefully.

Fixing this is easy: Only check if a cluster needs to be closed if a
cluster is in fact open.

Essentially the same happened with the tags in mkv_write_trailer(). It
has been fixed, too.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
I wonder whether we should rather check mkv->cluster_bc instead of
mkv->cluster_pos != -1. I would have made a commit to do so if I
understood this avio_write_marker stuff that seems be broken better (the
check below should probably be mkv->cluster_pos != -1 instead of != 0).

If opening a cluster fails, mkv->cluster_pos is still set; such problems
would be automatically fixed by such a switch. 

 libavformat/matroskaenc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 39ad5d1ccf..f84e50f94e 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -2354,6 +2354,7 @@ static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt)
     if (ret < 0)
         return ret;
 
+    if (mkv->cluster_pos != -1) {
     if (mkv->tracks[pkt->stream_index].write_dts)
         cluster_time = pkt->dts - mkv->cluster_pts;
     else
@@ -2381,9 +2382,10 @@ static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt)
         start_new_cluster = 0;
     }
 
-    if (mkv->cluster_pos != -1 && start_new_cluster) {
+    if (start_new_cluster) {
         mkv_end_cluster(s);
     }
+    }
 
     if (!mkv->cluster_pos)
         avio_write_marker(s->pb,
@@ -2498,7 +2500,7 @@ static int mkv_write_trailer(AVFormatContext *s)
         end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv);
 
         // update stream durations
-        if (!mkv->is_live) {
+        if (mkv->tags_bc) {
             int i;
             int64_t curr = avio_tell(mkv->tags_bc);
             for (i = 0; i < s->nb_streams; ++i) {
@@ -2522,8 +2524,6 @@ static int mkv_write_trailer(AVFormatContext *s)
                 }
             }
             avio_seek(mkv->tags_bc, curr, SEEK_SET);
-        }
-        if (mkv->tags_bc && !mkv->is_live) {
             avio_seek(pb, mkv->tags_pos, SEEK_SET);
             end_ebml_master_crc32(pb, &mkv->tags_bc, mkv);
         }
-- 
2.20.1