[FFmpeg-devel] [PATCH 12/20] avformat/matroskaenc: Improve Cues in case of no video
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Mon Apr 20 10:20:11 EEST 2020
Andreas Rheinhardt:
> The Matroska muxer currently only adds CuePoints in three cases:
> a) For video keyframes. b) For the first audio frame in a new Cluster if
> in DASH-mode. c) For subtitles. This means that ordinary Matroska audio
> files won't have any Cues which impedes seeking.
>
> This commit changes this. For every track in a file without video track
> it is checked and tracked whether a Cue entry has already been added
> for said track for the current Cluster. This is used to add a Cue entry
> for each first packet of each track in each Cluster.
>
> Implements #3149.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> libavformat/matroskaenc.c | 21 +++++++++++--------
> tests/ref/fate/aac-autobsf-adtstoasc | 4 ++--
> tests/ref/fate/matroska-flac-extradata-update | 4 ++--
> tests/ref/lavf/mka | 4 ++--
> 4 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index b9bfd34756..a469d48cc0 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -2120,6 +2120,10 @@ static void mkv_end_cluster(AVFormatContext *s)
> MatroskaMuxContext *mkv = s->priv_data;
>
> end_ebml_master_crc32(s->pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER, 0, 1);
> + if (!mkv->have_video) {
> + for (unsigned i = 0; i < s->nb_streams; i++)
> + mkv->tracks[i].has_cue = 0;
> + }
> mkv->cluster_pos = -1;
> avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_FLUSH_POINT);
> }
> @@ -2222,7 +2226,7 @@ static int mkv_check_new_extra_data(AVFormatContext *s, AVPacket *pkt)
> return 0;
> }
>
> -static int mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt, int add_cue)
> +static int mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt)
> {
> MatroskaMuxContext *mkv = s->priv_data;
> AVIOContext *pb;
> @@ -2268,10 +2272,12 @@ static int mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt, int add_
> ret = mkv_write_block(s, pb, MATROSKA_ID_SIMPLEBLOCK, pkt, keyframe);
> if (ret < 0)
> return ret;
> - if ((s->pb->seekable & AVIO_SEEKABLE_NORMAL) && (par->codec_type == AVMEDIA_TYPE_VIDEO && keyframe || add_cue)) {
> + if ((s->pb->seekable & AVIO_SEEKABLE_NORMAL) && keyframe &&
> + (par->codec_type == AVMEDIA_TYPE_VIDEO || !mkv->have_video && !track->has_cue)) {
> ret = mkv_add_cuepoint(mkv, pkt->stream_index, ts,
> mkv->cluster_pos, relative_packet_pos, -1);
> if (ret < 0) return ret;
> + track->has_cue = 1;
> }
> } else {
> if (par->codec_id == AV_CODEC_ID_WEBVTT) {
> @@ -2336,8 +2342,7 @@ static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt)
> // on seeing key frames.
> start_new_cluster = keyframe;
> } else if (mkv->is_dash && codec_type == AVMEDIA_TYPE_AUDIO &&
> - (mkv->cluster_pos == -1 ||
> - cluster_time > mkv->cluster_time_limit)) {
> + cluster_time > mkv->cluster_time_limit) {
> // For DASH audio, we create a Cluster based on cluster_time_limit
> start_new_cluster = 1;
> } else if (!mkv->is_dash &&
> @@ -2361,9 +2366,7 @@ static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt)
>
> // check if we have an audio packet cached
> if (mkv->cur_audio_pkt.size > 0) {
> - // for DASH audio, a CuePoint has to be added when there is a new cluster.
> - ret = mkv_write_packet_internal(s, &mkv->cur_audio_pkt,
> - mkv->is_dash ? start_new_cluster : 0);
> + ret = mkv_write_packet_internal(s, &mkv->cur_audio_pkt);
> av_packet_unref(&mkv->cur_audio_pkt);
> if (ret < 0) {
> av_log(s, AV_LOG_ERROR,
> @@ -2378,7 +2381,7 @@ static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt)
> if (pkt->size > 0)
> ret = av_packet_ref(&mkv->cur_audio_pkt, pkt);
> } else
> - ret = mkv_write_packet_internal(s, pkt, 0);
> + ret = mkv_write_packet_internal(s, pkt);
> return ret;
> }
>
> @@ -2406,7 +2409,7 @@ static int mkv_write_trailer(AVFormatContext *s)
>
> // check if we have an audio packet cached
> if (mkv->cur_audio_pkt.size > 0) {
> - ret = mkv_write_packet_internal(s, &mkv->cur_audio_pkt, 0);
> + ret = mkv_write_packet_internal(s, &mkv->cur_audio_pkt);
> if (ret < 0) {
> av_log(s, AV_LOG_ERROR,
> "Could not write cached audio packet ret:%d\n", ret);
> diff --git a/tests/ref/fate/aac-autobsf-adtstoasc b/tests/ref/fate/aac-autobsf-adtstoasc
> index f1c6f889d4..d9191fb37f 100644
> --- a/tests/ref/fate/aac-autobsf-adtstoasc
> +++ b/tests/ref/fate/aac-autobsf-adtstoasc
> @@ -1,5 +1,5 @@
> -9d0c81ce285a84c0137316004d091d95 *tests/data/fate/aac-autobsf-adtstoasc.matroska
> -6620 tests/data/fate/aac-autobsf-adtstoasc.matroska
> +76a14cc1b3292c7f724006d56b7e2eac *tests/data/fate/aac-autobsf-adtstoasc.matroska
> +6648 tests/data/fate/aac-autobsf-adtstoasc.matroska
> #extradata 0: 2, 0x0030001c
> #tb 0: 1/1000
> #media_type 0: audio
> diff --git a/tests/ref/fate/matroska-flac-extradata-update b/tests/ref/fate/matroska-flac-extradata-update
> index dfb2851b0f..16b268c4a8 100644
> --- a/tests/ref/fate/matroska-flac-extradata-update
> +++ b/tests/ref/fate/matroska-flac-extradata-update
> @@ -1,5 +1,5 @@
> -83aca2772c52f6f802cac288f889382b *tests/data/fate/matroska-flac-extradata-update.matroska
> -2019 tests/data/fate/matroska-flac-extradata-update.matroska
> +5f6a67a45906f1bc7dd11d840470b0e4 *tests/data/fate/matroska-flac-extradata-update.matroska
> +2071 tests/data/fate/matroska-flac-extradata-update.matroska
> #extradata 0: 34, 0x7acb09e7
> #extradata 1: 34, 0x7acb09e7
> #extradata 2: 34, 0x443402dd
> diff --git a/tests/ref/lavf/mka b/tests/ref/lavf/mka
> index b3c4117d92..24ccef51fd 100644
> --- a/tests/ref/lavf/mka
> +++ b/tests/ref/lavf/mka
> @@ -1,3 +1,3 @@
> -0d48d93057f14704f6b839bb15e7328a *tests/data/lavf/lavf.mka
> -43552 tests/data/lavf/lavf.mka
> +df7155d4333e9993c9ea2a9d53868881 *tests/data/lavf/lavf.mka
> +43580 tests/data/lavf/lavf.mka
> tests/data/lavf/lavf.mka CRC=0x3a1da17e
>
Given that this here was now on the ML for over two weeks and given that
Steve just confirmed to me via IRC that all outstanding Matroska muxer
patches are LGTM for him, I will merge them tomorrow if there are no
objections.
This means that if someone has better naming suggestions for the
FlagDefault option [1] he should say so now so that they can be considered.
- Andreas
[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-April/259802.html
More information about the ffmpeg-devel
mailing list