[FFmpeg-devel] [PATCH v2] avformat/matroskaenc: Write level 1 elements in one go
James Almer
jamrial at gmail.com
Wed Mar 25 18:11:09 EET 2020
On 3/25/2020 1:44 AM, Andreas Rheinhardt wrote:
> Up until now, writing level 1 elements proceeded as follows: First, the
> element id was written to the ordinary output AVIOContext and a dynamic
> buffer was opened for the content of the level 1 element in
> start_ebml_master_crc32(). Then this buffer was actually used and after it
> was closed (in end_ebml_master_crc32()), the size field corresponding to
> the buffer's size was written, after which the actual data was written.
>
> This commit changes this: Nothing is written to the main AVIOContext any
> more in start_ebml_master_crc32(). end_ebml_master_crc32() now writes
> both the id, the length field as well as the data. This implies that
> one can start a level 1 element in memory without outputting anything.
> This is done to enable to test whether enough space has been reserved
> for the Cues (if space has been reserved for them) before writing them.
> A large duration between outputting the header and outputting the rest
> could also break certain streaming usecases like the one from #8578
> (which this commit fixes).
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> The commit message has been updated in light of ticket #8578; nothing
> else has changed.
Should be ok. And please backport it to release/4.2 branch using git
cherry-pick -x, so the commit hash from master is referenced.
>
> I'd like to apply this and everything until (but excluding) #18 [1]
> in the coming days to fix the aforementioned issue (I will also backport
> a fix to 4.2).
>
> [1] is the cutoff point because upon rereading I am not sure anymore
> whether it is good to write the cues at the end if the reserved space
> doesn't suffice and return normally afterwards, because the fact that
> the cues have not been written at the front won't be detectable from the
> return value of av_write_trailer() any more (one only gets a
> AV_LOG_WARNING logmessage).
>
> [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-January/255157.html
>
> libavformat/matroskaenc.c | 60 +++++++++++++++++++++------------------
> 1 file changed, 32 insertions(+), 28 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 66f1bc4255..501d68a8f7 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -325,26 +325,26 @@ static void end_ebml_master(AVIOContext *pb, ebml_master master)
> avio_seek(pb, pos, SEEK_SET);
> }
>
> -static int start_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, MatroskaMuxContext *mkv,
> - uint32_t elementid)
> +static int start_ebml_master_crc32(AVIOContext **dyn_cp, MatroskaMuxContext *mkv)
> {
> int ret;
>
> if ((ret = avio_open_dyn_buf(dyn_cp)) < 0)
> return ret;
>
> - put_ebml_id(pb, elementid);
> if (mkv->write_crc)
> put_ebml_void(*dyn_cp, 6); /* Reserve space for CRC32 so position/size calculations using avio_tell() take it into account */
>
> return 0;
> }
>
> -static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, MatroskaMuxContext *mkv)
> +static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp,
> + MatroskaMuxContext *mkv, uint32_t id)
> {
> uint8_t *buf, crc[4];
> int size, skip = 0;
>
> + put_ebml_id(pb, id);
> size = avio_close_dyn_buf(*dyn_cp, &buf);
> put_ebml_num(pb, size, 0);
> if (mkv->write_crc) {
> @@ -362,13 +362,14 @@ static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, Matrosk
> * Complete ebml master without destroying the buffer, allowing for later updates
> */
> static void end_ebml_master_crc32_preliminary(AVIOContext *pb, AVIOContext **dyn_cp, MatroskaMuxContext *mkv,
> - int64_t *pos)
> + uint32_t id, int64_t *pos)
> {
> uint8_t *buf;
> int size = avio_get_dyn_buf(*dyn_cp, &buf);
>
> *pos = avio_tell(pb);
>
> + put_ebml_id(pb, id);
> put_ebml_num(pb, size, 0);
> avio_write(pb, buf, size);
> }
> @@ -450,7 +451,7 @@ static int mkv_write_seekhead(AVIOContext *pb, MatroskaMuxContext *mkv,
> goto seek;
> }
>
> - ret = start_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_SEEKHEAD);
> + ret = start_ebml_master_crc32(&dyn_cp, mkv);
> if (ret < 0)
> return ret;
>
> @@ -466,7 +467,7 @@ static int mkv_write_seekhead(AVIOContext *pb, MatroskaMuxContext *mkv,
> put_ebml_uint(dyn_cp, MATROSKA_ID_SEEKPOSITION, entry->segmentpos);
> end_ebml_master(dyn_cp, seekentry);
> }
> - end_ebml_master_crc32(pb, &dyn_cp, mkv);
> + end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_SEEKHEAD);
>
> remaining = seekhead->filepos + seekhead->reserved_size - avio_tell(pb);
> put_ebml_void(pb, remaining);
> @@ -510,7 +511,7 @@ static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tra
> int ret;
>
> currentpos = avio_tell(pb);
> - ret = start_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CUES);
> + ret = start_ebml_master_crc32(&dyn_cp, mkv);
> if (ret < 0)
> return ret;
>
> @@ -553,7 +554,7 @@ static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tra
> ffio_reset_dyn_buf(cuepoint);
> }
> ffio_free_dyn_buf(&cuepoint);
> - end_ebml_master_crc32(pb, &dyn_cp, mkv);
> + end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CUES);
>
> return currentpos;
> }
> @@ -1375,7 +1376,7 @@ static int mkv_write_tracks(AVFormatContext *s)
>
> mkv_add_seekhead_entry(mkv, MATROSKA_ID_TRACKS, avio_tell(pb));
>
> - ret = start_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, MATROSKA_ID_TRACKS);
> + ret = start_ebml_master_crc32(&mkv->tracks_bc, mkv);
> if (ret < 0)
> return ret;
>
> @@ -1390,9 +1391,10 @@ static int mkv_write_tracks(AVFormatContext *s)
> }
>
> if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live)
> - end_ebml_master_crc32_preliminary(pb, &mkv->tracks_bc, mkv, &mkv->tracks_pos);
> + end_ebml_master_crc32_preliminary(pb, &mkv->tracks_bc, mkv,
> + MATROSKA_ID_TRACKS, &mkv->tracks_pos);
> else
> - end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv);
> + end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, MATROSKA_ID_TRACKS);
>
> return 0;
> }
> @@ -1410,7 +1412,7 @@ static int mkv_write_chapters(AVFormatContext *s)
>
> mkv_add_seekhead_entry(mkv, MATROSKA_ID_CHAPTERS, avio_tell(pb));
>
> - ret = start_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CHAPTERS);
> + ret = start_ebml_master_crc32(&dyn_cp, mkv);
> if (ret < 0) return ret;
>
> editionentry = start_ebml_master(dyn_cp, MATROSKA_ID_EDITIONENTRY, 0);
> @@ -1448,7 +1450,7 @@ static int mkv_write_chapters(AVFormatContext *s)
> end_ebml_master(dyn_cp, chapteratom);
> }
> end_ebml_master(dyn_cp, editionentry);
> - end_ebml_master_crc32(pb, &dyn_cp, mkv);
> + end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CHAPTERS);
>
> mkv->wrote_chapters = 1;
> return 0;
> @@ -1499,7 +1501,7 @@ static int mkv_write_tag_targets(AVFormatContext *s, uint32_t elementid,
> if (!mkv->tags_bc) {
> mkv_add_seekhead_entry(mkv, MATROSKA_ID_TAGS, avio_tell(s->pb));
>
> - ret = start_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, MATROSKA_ID_TAGS);
> + ret = start_ebml_master_crc32(&mkv->tags_bc, mkv);
> if (ret < 0)
> return ret;
> }
> @@ -1644,9 +1646,10 @@ static int mkv_write_tags(AVFormatContext *s)
>
> if (mkv->tags_bc) {
> if ((s->pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live)
> - end_ebml_master_crc32_preliminary(s->pb, &mkv->tags_bc, mkv, &mkv->tags_pos);
> + end_ebml_master_crc32_preliminary(s->pb, &mkv->tags_bc, mkv,
> + MATROSKA_ID_TAGS, &mkv->tags_pos);
> else
> - end_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv);
> + end_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, MATROSKA_ID_TAGS);
> }
> return 0;
> }
> @@ -1669,7 +1672,7 @@ static int mkv_write_attachments(AVFormatContext *s)
>
> mkv_add_seekhead_entry(mkv, MATROSKA_ID_ATTACHMENTS, avio_tell(pb));
>
> - ret = start_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_ATTACHMENTS);
> + ret = start_ebml_master_crc32(&dyn_cp, mkv);
> if (ret < 0) return ret;
>
> for (i = 0; i < s->nb_streams; i++) {
> @@ -1742,7 +1745,7 @@ static int mkv_write_attachments(AVFormatContext *s)
> mkv->attachments->entries[mkv->attachments->num_entries].stream_idx = i;
> mkv->attachments->entries[mkv->attachments->num_entries++].fileuid = fileuid;
> }
> - end_ebml_master_crc32(pb, &dyn_cp, mkv);
> + end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_ATTACHMENTS);
>
> return 0;
> }
> @@ -1821,7 +1824,7 @@ static int mkv_write_header(AVFormatContext *s)
>
> mkv_add_seekhead_entry(mkv, MATROSKA_ID_INFO, avio_tell(pb));
>
> - ret = start_ebml_master_crc32(pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO);
> + ret = start_ebml_master_crc32(&mkv->info_bc, mkv);
> if (ret < 0)
> return ret;
> pb = mkv->info_bc;
> @@ -1880,9 +1883,10 @@ static int mkv_write_header(AVFormatContext *s)
> }
> }
> if ((s->pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live)
> - end_ebml_master_crc32_preliminary(s->pb, &mkv->info_bc, mkv, &mkv->info_pos);
> + end_ebml_master_crc32_preliminary(s->pb, &mkv->info_bc, mkv,
> + MATROSKA_ID_INFO, &mkv->info_pos);
> else
> - end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv);
> + end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO);
> pb = s->pb;
>
> ret = mkv_write_tracks(s);
> @@ -2170,7 +2174,7 @@ static void mkv_end_cluster(AVFormatContext *s)
> {
> MatroskaMuxContext *mkv = s->priv_data;
>
> - end_ebml_master_crc32(s->pb, &mkv->cluster_bc, mkv);
> + end_ebml_master_crc32(s->pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER);
> mkv->cluster_pos = -1;
> avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_FLUSH_POINT);
> }
> @@ -2311,7 +2315,7 @@ static int mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt, int add_
>
> if (mkv->cluster_pos == -1) {
> mkv->cluster_pos = avio_tell(s->pb);
> - ret = start_ebml_master_crc32(s->pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER);
> + ret = start_ebml_master_crc32(&mkv->cluster_bc, mkv);
> if (ret < 0)
> return ret;
> put_ebml_uint(mkv->cluster_bc, MATROSKA_ID_CLUSTERTIMECODE, FFMAX(0, ts));
> @@ -2477,7 +2481,7 @@ static int mkv_write_trailer(AVFormatContext *s)
> }
>
> if (mkv->cluster_bc) {
> - end_ebml_master_crc32(pb, &mkv->cluster_bc, mkv);
> + end_ebml_master_crc32(pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER);
> }
>
> ret = mkv_write_chapters(s);
> @@ -2525,11 +2529,11 @@ static int mkv_write_trailer(AVFormatContext *s)
> av_log(s, AV_LOG_DEBUG, "end duration = %" PRIu64 "\n", mkv->duration);
> avio_seek(mkv->info_bc, mkv->duration_offset, SEEK_SET);
> put_ebml_float(mkv->info_bc, MATROSKA_ID_DURATION, mkv->duration);
> - end_ebml_master_crc32(pb, &mkv->info_bc, mkv);
> + end_ebml_master_crc32(pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO);
>
> // write tracks master
> avio_seek(pb, mkv->tracks_pos, SEEK_SET);
> - end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv);
> + end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, MATROSKA_ID_TRACKS);
>
> // update stream durations
> if (!mkv->is_live) {
> @@ -2559,7 +2563,7 @@ static int mkv_write_trailer(AVFormatContext *s)
> }
> if (mkv->tags_bc && !mkv->is_live) {
> avio_seek(pb, mkv->tags_pos, SEEK_SET);
> - end_ebml_master_crc32(pb, &mkv->tags_bc, mkv);
> + end_ebml_master_crc32(pb, &mkv->tags_bc, mkv, MATROSKA_ID_TAGS);
> }
>
> avio_seek(pb, currentpos, SEEK_SET);
>
More information about the ffmpeg-devel
mailing list