On Sat Apr 4 09:29:20 EEST 2020, Andreas Rheinhardt
<andreas.rheinha...@gmail.com> wrote:
> Are you running into this limitation?
Yes.
> If so, do the files that you
> create with this patch that have more than 127 tracks work? They
> shouldn't. The reason for this (and the reason that I didn't remove this
> limit altogether in 65ef74f74900590f134b4a130e8f56e5272d1925) lies in
> the way the TrackNumber is encoded in the (Simple)Blocks: They are
> encoded as variable-length numbers, so that encoding small TrackNumbers
> takes up less bytes than encoding bigger TrackNumbers. More precisely,
> TrackNumbers in the range 1..127 are encodable on one byte. And the way
> they are written in mkv_write_block() and mkv_write_vtt_blocks() relies
> on this. If you simply remove said limit, the tracks with TrackNumbers >
> 127 will not have any (Simple)Blocks associated with them; instead the
> encoded TrackNumber will be the desired TrackNumber modulo 128 and the
> (Simple)Block will appear to belong to the track with the encoded
> TrackNumber (if one exists).** The results will of course be catastrophic.
I skimmed through libmatroska's code in src/KaxBlock.cpp and their
limit is 0x4000-1, which is way more reasonable.
> Notice that I have sent a patchset that slightly increases the number of
> allowed tracks (without fixing the root cause though, because I didn't
> know if there are really people who run into this): [1] makes
> attachments no longer count towards the limit; [2] increases the limit
> to 127. I will resend said patchset soon.
>
> - Andreas
>
> [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-November/253452.html
> [2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-November/252563.html
>
> *: The reason for setting MAX_TRACKS to 126 instead of 127 is probably
> that the encoding corresponding to the number 127 is special when
> encoding lengths (which use the same variable-length number scheme):
> Here it denotes an unknown length instead of a length of 127. But there
> are no such values with a special meaning for encoded TrackNumbers.
Indeed, libmatroska allows 0x80-1 (127) per byte. Your second patch
should have been merged.
> **: Notice that a TrackNumber of 0 is invalid, so any (Simple)Block that
> ought to belong to the tracks with TrackNumber 128, 256, 384, ... will
> have no matching track at all. FFmpeg's Matroska demuxer treats
> encountering such (Simple)Blocks as a sign of invalid data and it will
> try to resync to the next level 1 element (typically the next Cluster).
> Similar things can happen if you use attachments (in this case there may
> be gaps in the used TrackNumbers).
I have included a new patch, which writes the track_number using
put_ebml_num(), which should work perhaps?
From 477d1ee08115e09f2f61f75dd70ce7bc882d2c34 Mon Sep 17 00:00:00 2001
From: Jan Chren (rindeal) <dev.rind...@gmail.com>
Date: Sat, 4 Apr 2020 00:00:00 +0000
Subject: [PATCH] avformat/matroskaenc: remove track limit
Strictly speaking, ebml_num_size() wastes one bit here.
Employing new ebml_num_size() and put_ebml_num() just for track_number
would solve this.
---
libavformat/matroskaenc.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 5dae53026d..30132a53e8 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -117,10 +117,6 @@ typedef struct mkv_attachments {
#define MODE_MATROSKAv2 0x01
#define MODE_WEBM 0x02
-/** Maximum number of tracks allowed in a Matroska file (with track numbers in
- * range 1 to 126 (inclusive) */
-#define MAX_TRACKS 126
-
typedef struct MatroskaMuxContext {
const AVClass *class;
int mode;
@@ -2089,9 +2085,8 @@ static int mkv_write_block(AVFormatContext *s, AVIOContext *pb,
}
put_ebml_id(pb, blockid);
- put_ebml_num(pb, size + 4, 0);
- // this assumes stream_index is less than 126
- avio_w8(pb, 0x80 | track_number);
+ put_ebml_num(pb, size + ebml_num_size(track_number) + 2 + 1, 0);
+ put_ebml_num(track_number, 0);
avio_wb16(pb, ts - mkv->cluster_pts);
avio_w8(pb, (blockid == MATROSKA_ID_SIMPLEBLOCK && keyframe) ? (1 << 7) : 0);
avio_write(pb, data + offset, size);
@@ -2155,8 +2150,8 @@ static int mkv_write_vtt_blocks(AVFormatContext *s, AVIOContext *pb, AVPacket *p
blockgroup = start_ebml_master(pb, MATROSKA_ID_BLOCKGROUP, mkv_blockgroup_size(size));
put_ebml_id(pb, MATROSKA_ID_BLOCK);
- put_ebml_num(pb, size + 4, 0);
- avio_w8(pb, 0x80 | (pkt->stream_index + 1)); // this assumes stream_index is less than 126
+ put_ebml_num(pb, size + ebml_num_size(pkt->stream_index + 1) + 2 + 1, 0);
+ put_ebml_num(pkt->stream_index + 1, 0);
avio_wb16(pb, ts - mkv->cluster_pts);
avio_w8(pb, flags);
avio_printf(pb, "%.*s\n%.*s\n%.*s", id_size, id, settings_size, settings, pkt->size, pkt->data);
@@ -2604,13 +2599,6 @@ static int mkv_init(struct AVFormatContext *s)
{
int i;
- if (s->nb_streams > MAX_TRACKS) {
- av_log(s, AV_LOG_ERROR,
- "At most %d streams are supported for muxing in Matroska\n",
- MAX_TRACKS);
- return AVERROR(EINVAL);
- }
-
for (i = 0; i < s->nb_streams; i++) {
if (s->streams[i]->codecpar->codec_id == AV_CODEC_ID_ATRAC3 ||
s->streams[i]->codecpar->codec_id == AV_CODEC_ID_COOK ||
--
2.26.0
_______________________________________________
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".
