diff mbox series

[FFmpeg-devel,30/30] avformat/matroskaenc: Improve BlockAdditions

Message ID 20200126051028.27455-2-andreas.rheinhardt@gmail.com
State Accepted
Headers show
Series Matroska muxer patches
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt Jan. 26, 2020, 5:10 a.m. UTC
8ffcc826 added support for muxing BlockAdditions with BlockAddID equal
to one. The restriction to BlockAddID == 1 probably resulted from a
limitation to what was needed; yet over time this led to three
occurences of "(side_data_size && additional_id == 1)". This commit
changes this by setting side_data_size to 0 if additional_id != 1.

It also stops hardcoding 1 for the value of BlockAddID to write;
but it still upholds the requirement that it is 1. See below.

Despite BlockAddID actually having a default value of 1, it is still
written, because until very recently (namely dbc50f8a) our demuxer
used a wrong default value of 0.

Furthermore, use put_ebml_binary() to write the BlockAdditional element.

(The Matroska specifications have evolved and now the BlockAddID 1 is
reserved for the codec (as described in the codec's codec mapping),
BlockMore elements with BlockAddID > 1 are now of a more
codec-independent nature and require a BlockAdditionalMapping in the
track's TrackEntry. Given that this muxer does not support writing said
BlockAdditionalMapping yet (actually, none have been defined yet), we
have to uphold the requirement that BlockAddID == 1.)

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
Btw: When writing BlockAdditions in Matroska mode, one also has to write
the MaxBlockAdditionID in the corresponding TrackEntry. Yet we don't
(and we don't have the information to do that reliably when writing the
header).

 libavformat/matroskaenc.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 8fc672a31f..de3fff2043 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -2046,7 +2046,7 @@  static void mkv_write_block(AVFormatContext *s, AVIOContext *pb,
     uint8_t *data = NULL, *side_data = NULL;
     int offset = 0, size = pkt->size, side_data_size = 0;
     int64_t ts = track->write_dts ? pkt->dts : pkt->pts;
-    uint64_t additional_id = 0;
+    uint64_t additional_id;
     int64_t discard_padding = 0;
     uint8_t track_number = (mkv->is_dash ? mkv->dash_track_number : (pkt->stream_index + 1));
     ebml_master block_group, block_additions, block_more;
@@ -2100,16 +2100,16 @@  static void mkv_write_block(AVFormatContext *s, AVIOContext *pb,
                                         AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
                                         &side_data_size);
     if (side_data) {
-        if (side_data_size < 8) {
+        // Only the Codec-specific BlockMore (id == 1) is currently supported.
+        if (side_data_size < 8 || (additional_id = AV_RB64(side_data)) != 1) {
             side_data_size = 0;
         } else {
-            additional_id   = AV_RB64(side_data);
             side_data      += 8;
             side_data_size -= 8;
         }
     }
 
-    if ((side_data_size && additional_id == 1) || discard_padding) {
+    if (side_data_size || discard_padding) {
         block_group = start_ebml_master(pb, MATROSKA_ID_BLOCKGROUP, 0);
         blockid = MATROSKA_ID_BLOCK;
     }
@@ -2133,17 +2133,18 @@  static void mkv_write_block(AVFormatContext *s, AVIOContext *pb,
         put_ebml_sint(pb, MATROSKA_ID_DISCARDPADDING, discard_padding);
     }
 
-    if (side_data_size && additional_id == 1) {
+    if (side_data_size) {
         block_additions = start_ebml_master(pb, MATROSKA_ID_BLOCKADDITIONS, 0);
         block_more = start_ebml_master(pb, MATROSKA_ID_BLOCKMORE, 0);
-        put_ebml_uint(pb, MATROSKA_ID_BLOCKADDID, 1);
-        put_ebml_id(pb, MATROSKA_ID_BLOCKADDITIONAL);
-        put_ebml_num(pb, side_data_size, 0);
-        avio_write(pb, side_data, side_data_size);
+        /* Until dbc50f8a our demuxer used a wrong default value
+         * of BlockAddID, so we write it unconditionally. */
+        put_ebml_uint  (pb, MATROSKA_ID_BLOCKADDID, additional_id);
+        put_ebml_binary(pb, MATROSKA_ID_BLOCKADDITIONAL,
+                        side_data, side_data_size);
         end_ebml_master(pb, block_more);
         end_ebml_master(pb, block_additions);
     }
-    if ((side_data_size && additional_id == 1) || discard_padding) {
+    if (side_data_size || discard_padding) {
         end_ebml_master(pb, block_group);
     }
 }