Andreas Rheinhardt:
> When the user opted to write the Cues at the beginning, the Cues were
> simply written without checking in advance whether enough space has been
> reserved for them. If it wasn't enough, the data following the space
> reserved for the Cues was simply overwritten, corrupting the file.
>
> This commit changes this by checking whether enough space has been
> reserved for the Cues before outputting anything. If it isn't enough,
> no Cues will be output at all and the file will be finalized normally,
> yet writing the trailer will nevertheless return an error to notify
> the user that his wish of having Cues at the front of the file hasn't
> been fulfilled.
>
> This change opens new usecases for this option: It is now safe to use
> this option to e.g. record live streams or to use it when muxing the
> output of an expensive encoding, because when the reserved space turns
> out to be insufficient, one ends up with a file that just lacks Cues
> but is otherwise fine.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
> ---
> v2: Now returning an error if the space reserved for the Cues does not
> suffice. (The earlier version had the drawback that the return value did
> not inform them of failure to write the Cues at the front due to the
> insufficient space reserved.) Apart from that the file is finalized normally.
>
> I intend to apply this soon (tomorrow) if no one objects.
>
> doc/muxers.texi | 5 ++-
> libavformat/matroskaenc.c | 86 +++++++++++++++++++++------------------
> 2 files changed, 50 insertions(+), 41 deletions(-)
>
> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index d304181671..3be1c89416 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -1352,8 +1352,9 @@ index at the beginning of the file.
>
> If this option is set to a non-zero value, the muxer will reserve a given
> amount
> of space in the file header and then try to write the cues there when the
> muxing
> -finishes. If the available space does not suffice, muxing will fail. A safe
> size
> -for most use cases should be about 50kB per hour of video.
> +finishes. If the reserved space does not suffice, no Cues will be written,
> the
> +file will be finalized and writing the trailer will return an error.
> +A safe size for most use cases should be about 50kB per hour of video.
>
> Note that cues are only written if the output is seekable and this option
> will
> have no effect if it is not.
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 5dae53026d..22cc693720 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -500,23 +500,15 @@ static int mkv_add_cuepoint(MatroskaMuxContext *mkv,
> int stream, int tracknum, i
> return 0;
> }
>
> -static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track
> *tracks, int num_tracks)
> +static int mkv_assemble_cues(AVStream **streams, AVIOContext *dyn_cp,
> + mkv_cues *cues, mkv_track *tracks, int
> num_tracks)
> {
> - MatroskaMuxContext *mkv = s->priv_data;
> - AVIOContext *dyn_cp, *pb = s->pb, *cuepoint;
> - int64_t currentpos;
> + AVIOContext *cuepoint;
> int ret;
>
> - currentpos = avio_tell(pb);
> - ret = start_ebml_master_crc32(&dyn_cp, mkv);
> - if (ret < 0)
> - return ret;
> -
> ret = avio_open_dyn_buf(&cuepoint);
> - if (ret < 0) {
> - ffio_free_dyn_buf(&dyn_cp);
> + if (ret < 0)
> return ret;
> - }
>
> for (mkv_cuepoint *entry = cues->entries, *end = entry +
> cues->num_entries;
> entry < end;) {
> @@ -535,7 +527,7 @@ static int64_t mkv_write_cues(AVFormatContext *s,
> mkv_cues *cues, mkv_track *tra
> int idx = entry->stream_idx;
>
> av_assert0(idx >= 0 && idx < num_tracks);
> - if (tracks[idx].has_cue && s->streams[idx]->codecpar->codec_type
> != AVMEDIA_TYPE_SUBTITLE)
> + if (tracks[idx].has_cue && streams[idx]->codecpar->codec_type !=
> AVMEDIA_TYPE_SUBTITLE)
> continue;
> tracks[idx].has_cue = 1;
> track_positions = start_ebml_master(cuepoint,
> MATROSKA_ID_CUETRACKPOSITION, MAX_CUETRACKPOS_SIZE);
> @@ -551,9 +543,8 @@ 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, MATROSKA_ID_CUES);
>
> - return currentpos;
> + return 0;
> }
>
> static int put_xiph_codecpriv(AVFormatContext *s, AVIOContext *pb,
> AVCodecParameters *par)
> @@ -2464,7 +2455,7 @@ static int mkv_write_trailer(AVFormatContext *s)
> {
> MatroskaMuxContext *mkv = s->priv_data;
> AVIOContext *pb = s->pb;
> - int64_t currentpos, cuespos;
> + int64_t currentpos;
> int ret;
>
> // check if we have an audio packet cached
> @@ -2487,35 +2478,52 @@ static int mkv_write_trailer(AVFormatContext *s)
>
>
> if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live) {
> + int64_t ret64;
> +
> if (mkv->cues.num_entries) {
> - if (mkv->reserve_cues_space) {
> - int64_t cues_end;
> -
> - currentpos = avio_tell(pb);
> - avio_seek(pb, mkv->cues_pos, SEEK_SET);
> -
> - cuespos = mkv_write_cues(s, &mkv->cues, mkv->tracks,
> s->nb_streams);
> - cues_end = avio_tell(pb);
> - if (cues_end > cuespos + mkv->reserve_cues_space) {
> - av_log(s, AV_LOG_ERROR,
> - "Insufficient space reserved for cues: %d "
> - "(needed: %" PRId64 ").\n",
> - mkv->reserve_cues_space, cues_end - cuespos);
> - return AVERROR(EINVAL);
> - }
> + AVIOContext *cues;
> + uint64_t size;
>
> - if (cues_end < cuespos + mkv->reserve_cues_space)
> - put_ebml_void(pb, mkv->reserve_cues_space -
> - (cues_end - cuespos));
> + ret = start_ebml_master_crc32(&cues, mkv);
> + if (ret < 0)
> + return ret;
>
> - avio_seek(pb, currentpos, SEEK_SET);
> - } else {
> - cuespos = mkv_write_cues(s, &mkv->cues, mkv->tracks,
> s->nb_streams);
> + ret = mkv_assemble_cues(s->streams, cues, &mkv->cues,
> + mkv->tracks, s->nb_streams);
> + if (ret < 0) {
> + ffio_free_dyn_buf(&cues);
> + return ret;
> }
>
> - mkv_add_seekhead_entry(mkv, MATROSKA_ID_CUES, cuespos);
> + if (mkv->reserve_cues_space) {
> + size = avio_tell(cues);
> + size += 4 + ebml_num_size(size);
> + if (mkv->reserve_cues_space < size) {
> + av_log(s, AV_LOG_WARNING,
> + "Insufficient space reserved for Cues: "
> + "%d < %"PRIu64". No Cues will be output.\n",
> + mkv->reserve_cues_space, size);
> + mkv->reserve_cues_space = -1;
> + ffio_free_dyn_buf(&cues);
> + goto after_cues;
> + } else {
> + currentpos = avio_tell(pb);
> + if ((ret64 = avio_seek(pb, mkv->cues_pos, SEEK_SET)) <
> 0) {
> + ffio_free_dyn_buf(&cues);
> + return ret64;
> + }
> + }
> + }
> + mkv_add_seekhead_entry(mkv, MATROSKA_ID_CUES, avio_tell(pb));
> + end_ebml_master_crc32(pb, &cues, mkv, MATROSKA_ID_CUES);
> + if (mkv->reserve_cues_space) {
> + if (size < mkv->reserve_cues_space)
> + put_ebml_void(pb, mkv->reserve_cues_space - size);
> + avio_seek(pb, currentpos, SEEK_SET);
> + }
> }
>
> + after_cues:
> currentpos = avio_tell(pb);
>
> ret = mkv_write_seekhead(pb, mkv, 1, mkv->info_pos);
> @@ -2570,7 +2578,7 @@ static int mkv_write_trailer(AVFormatContext *s)
> end_ebml_master(pb, mkv->segment);
> }
>
> - return 0;
> + return mkv->reserve_cues_space < 0 ? AVERROR(EINVAL) : 0;
> }
>
> static int mkv_query_codec(enum AVCodecID codec_id, int std_compliance)
>
Applied these three.
- Andreas
_______________________________________________
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".
