From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 9ECD647A33 for ; Fri, 29 Sep 2023 16:08:37 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C597668C9A6; Fri, 29 Sep 2023 19:08:34 +0300 (EEST) Received: from mail-pf1-f172.google.com (mail-pf1-f172.google.com [209.85.210.172]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 8E9FC68C9A6 for ; Fri, 29 Sep 2023 19:08:28 +0300 (EEST) Received: by mail-pf1-f172.google.com with SMTP id d2e1a72fcca58-690d8fb3b7eso12878058b3a.1 for ; Fri, 29 Sep 2023 09:08:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1696003706; x=1696608506; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:autocrypt:from :content-language:references:to:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=8HfsrT2amjlyLzabx96/oqTZRCU5s6Yjt9gUfoQQX78=; b=gIR3YEcQckUG92TirQPCVPhx4dxGOA1HaJ9nWyVVM7nECbTdTdstHJ2uEnwmvrmwCM gPZqlrKYQKSi38TozFYBVxlIV1noUqrLruCr+VmLvhh31E8QXM5s4h811yHUHgrLWFGV QmnLUD1eXrhx/QL58BfUHAbvDPY649ue20zrqksbsFZfUBz1Pxwl9Zp6QrXiD+o79HV+ Hzr7zHEkeEWOCpUMg1UPdBhQslwh3tb7GYQZZn8eiPi9lnFZsP9+r4bkiF/awTS2YN5c qWz/Gjvszh+GgP/LFuEGAQX2zaBhUuMVwG3c2doSNLECIPS0poZxD1IgiWLZx088ZJM+ z6xA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696003706; x=1696608506; h=content-transfer-encoding:in-reply-to:autocrypt:from :content-language:references:to:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=8HfsrT2amjlyLzabx96/oqTZRCU5s6Yjt9gUfoQQX78=; b=buuz73UzEsLQ8KpTM8sw6ISd5QJpEHZ1KyM/0R3jd0DVecJnCePYlu01l38PjEhjBW 0I2htYUTVP9RU5HwBFKlVvuWvW67RWTmCijZY7akMex4dv3wRq0XxJv2D4s5oRfWvxiC EQcppRtv2bzztGKwVGyqBnp8CSAMiwhnWYoL8+pHhLEyR3q0XD/Uvg3WGu4mtNVfHvyk WOcrpXBspeP6I8/+sv9DDFGFfgUSAxxKBw2Lb1kEGdY+oYj+FaG8wInFTrWF9elm7QZS +nmf5GLK/0CCvRUwCGFPG3fXz7Oz9rKWHeOUox3kV7tdtib2bwW1HBtK5J3mkCnCqHU+ hHoA== X-Gm-Message-State: AOJu0YyBW69A/rkoGuMownyp2HcKQw75SytGxNmsUvfEtNsFuR5av4wQ aWAY3m+svuXan4oJGs60VQPtyPsdvVU= X-Google-Smtp-Source: AGHT+IFzFRdMJoUw3ZAGuLFuHza2+xNQIUn01/ByHTSUcg8zHzvkA+ozJS74vLbGiy1QoliBADyRDA== X-Received: by 2002:a05:6a20:4284:b0:162:4f45:b415 with SMTP id o4-20020a056a20428400b001624f45b415mr5318263pzj.51.1696003705661; Fri, 29 Sep 2023 09:08:25 -0700 (PDT) Received: from [192.168.0.10] (host197.190-225-105.telecom.net.ar. [190.225.105.197]) by smtp.gmail.com with ESMTPSA id z22-20020aa791d6000000b0068a0c403636sm15122236pfa.192.2023.09.29.09.08.24 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 29 Sep 2023 09:08:25 -0700 (PDT) Message-ID: <181aecfd-986e-4976-bfdb-ef29e3bd1e4e@gmail.com> Date: Fri, 29 Sep 2023 13:08:28 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: Content-Language: en-US From: James Almer Autocrypt: addr=jamrial@gmail.com; keydata= xsBNBFjZtqABCADLW+vdEoZaJZDsIO6geYFTOcn1unsEHefj9zn+3oTHlDFFzO47mzHsSfbK 9JE2xpOJEVnC8FAF5Sayi/pVwV+mtQUV3n5dgVeVBYF9GUQwOGFCpK8X54RRqhkgknbunOEE 0CtgAJgmpFmmmHgq02GvEspx1h/rh4apqwQR6QX4Favb+x9+i9ytVpwVcBX94vo2toyP7h/K BWfadQmb8ltgE1kshfg+SQs/H5bTV5Z1DuEASf02ZL/1qYB/sdTgWPLv9XMUHHsRFmMY8TMx wJSkP+Af3AiYQPJYz1B1D4tt98T/NoiVdin10zATakPjV8hXaobuRmxgakkUASXudydDABEB AAHNH0phbWVzIEFsbWVyIDxqYW1yaWFsQGdtYWlsLmNvbT7CwJIEEwEIADwCGwMGCwkIBwMC BhUIAgkKCwQWAgMBAh4BAheAFiEEd1EujP2UoWlX5pp6FGMBrXN2WeAFAmJoLUUCGQEACgkQ FGMBrXN2WeAFVQf9GtGhniRs1PzNUOgJktCnv6j4BbLieaIPYPEFXKDHOgjqQE2zVMYXnoXl Jam928ii902a8OY06r9ywn/R8ApD1/3NY/v64O71CY9scz5XyH2au8wIZ6HwFy3/f7sqjdGD uctY8Qs7rjT7NkoC5lmgMu2v2k03dGtM9AAf5AK5gU+H0EUw7vmKKiXzUqt5kvBuf4CEwXvH AQT1SMJ52rIlDWB7FQFyZeUbOAK2IgY/KNedfK6nsgd/eQVnlofPd2XoddE7kP6iys7jJefw DD3g3rZyDTq7in5dyk5glaNpWZpbHGBs+9SCYLnfQ8XvWqPFOD+gj0plamKANgOvavKTxM7A TQRY2bagAQgA69YtILj8kYxmqPr/M8+MXT7wVoOWVW9lvSmPquCELaDy/NIS7D06VC5EuE/6 JlJXZMTn37NLlyWhzwOgXuXw5w2tyoQQBuvqGiXJijuXwXH7HKdzrc6rpYtAqt5w05hzNrFS KrS0izG64VpWrfproy3BsL+8TBm9brLhhNPynVRqVukbbGzlATTzNQGZ14TTi2/dL6DkMQnM qn4jX9UEe4GdGQBP50bUJSSmeiIkyNLWA+znuN2PZEz930ZwNrF9GtDVw7mzcmpCZ7spldE2 tutbpy9D1bIqxyqBrYDSezyzL2adR1qgHyOTMCHg2AYNkrIQHrSyJxKTpZ1/hqOp8wARAQAB wsBfBBgBAgAJBQJY2bagAhsMAAoJEBRjAa1zdlnghekH/0Yb0iYJ74oID2f/Fj+AJKS2ekQF P2xOr8lpGzgp/+yWUvPtqbX0A33anBJdYwxaAC0NataX3tfZ+oJkzXqfmqhIHMPYHdZesJA2 Bk9hU/33mDl5s5U66/z0uelWzwKVHoQ2O6or4+qF3HJFSJLCe9uvWJ3zXf9F342Ftj73sfx+ 3xkw/IXsN1RqbYqDlzpoEQ99SIEfY/8Jjwnd3sIPfqkuyeaYfe6GJDqKawdCEP1oRRlbXEAp TJgYz8r3nPhGv9cdHNDCk44ISbsqVuxIEnLqi4fTPZaGupiQhT+srl268TTAp2TQW7+6Ce/b NPQorMquzS/LZoyALpmsYi/miMc= In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH 1/2] avformat/matroskaenc: Don't create wrong packet durations X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 9/29/2023 1:01 PM, Andreas Rheinhardt wrote: > We have to write an explicit BlockDuration element (and use > a BlockGroup instead of a SimpleBlock) in case the Track > has a DefaultDuration that is inconsistent with the duration > of the packet. > > The matroska-h264-remux test uses a file with coded fields > where the duration of a Block is the duration of a field, > not of a frame, therefore this patch writes said BlockDuration > elements. > > (When using a BlockGroup, one has to add ReferenceBlock elements > to distinguish keyframes from non-keyframes. Unfortunately, > the AV1 codec mapping [1] requires us to reference all references > and to really use the real references, which requires a lot of > effort for basically no gain. When BlockGroups are used with AV1, > the created files are most likely invalid, both before and after > this patch, but this patch makes this more likely to happen.) AV1 has no fields, and the vast majority of samples will also have fixed frame duration across the entire coded stream, so i doubt this will make a difference. There's also the fact it's unlikely parsers will care about ReferenceBlock elements at all to begin with, and instead just have the decoder handle things relying on the bitstream level information. > > [1]: https://github.com/ietf-wg-cellar/matroska-specification/blob/master/codec/av1.md > > Signed-off-by: Andreas Rheinhardt > --- > libavformat/matroskaenc.c | 40 ++++++++++++++++++++++-------- > tests/ref/fate/matroska-h264-remux | 4 +-- > 2 files changed, 32 insertions(+), 12 deletions(-) > > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > index ba54f5f98e..9286932a08 100644 > --- a/libavformat/matroskaenc.c > +++ b/libavformat/matroskaenc.c > @@ -195,6 +195,8 @@ typedef struct mkv_track { > int codecpriv_offset; > unsigned codecpriv_size; ///< size reserved for CodecPrivate excluding header+length field > int64_t ts_offset; > + uint64_t default_duration_low; > + uint64_t default_duration_high; > /* This callback will be called twice: First with a NULL AVIOContext > * to return the size of the (Simple)Block's data via size > * and a second time with the AVIOContext set when the data > @@ -1805,6 +1807,16 @@ static int mkv_write_track_video(AVFormatContext *s, MatroskaMuxContext *mkv, > return ebml_writer_write(&writer, pb); > } > > +static void mkv_write_default_duration(mkv_track *track, AVIOContext *pb, > + AVRational duration) > +{ > + put_ebml_uint(pb, MATROSKA_ID_TRACKDEFAULTDURATION, > + 1000000000LL * duration.num / duration.den); > + track->default_duration_low = 1000LL * duration.num / duration.den; > + track->default_duration_high = track->default_duration_low + > + !!(1000LL * duration.num % duration.den); > +} > + > static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv, > AVStream *st, mkv_track *track, AVIOContext *pb, > int is_default) > @@ -1913,16 +1925,19 @@ static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv, > } > > switch (par->codec_type) { > + AVRational frame_rate; > case AVMEDIA_TYPE_VIDEO: > mkv->have_video = 1; > put_ebml_uint(pb, MATROSKA_ID_TRACKTYPE, MATROSKA_TRACK_TYPE_VIDEO); > > - if( st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0 > - && av_cmp_q(av_inv_q(st->avg_frame_rate), st->time_base) > 0) > - put_ebml_uint(pb, MATROSKA_ID_TRACKDEFAULTDURATION, 1000000000LL * st->avg_frame_rate.den / st->avg_frame_rate.num); > - else if( st->r_frame_rate.num > 0 && st->r_frame_rate.den > 0 > - && av_cmp_q(av_inv_q(st->r_frame_rate), st->time_base) > 0) > - put_ebml_uint(pb, MATROSKA_ID_TRACKDEFAULTDURATION, 1000000000LL * st->r_frame_rate.den / st->r_frame_rate.num); > + frame_rate = (AVRational){ 0, 1 }; > + if (st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0) > + frame_rate = st->avg_frame_rate; > + else if (st->r_frame_rate.num > 0 && st->r_frame_rate.den > 0) > + frame_rate = st->r_frame_rate; > + > + if (frame_rate.num > 0) > + mkv_write_default_duration(track, pb, av_inv_q(frame_rate)); > > if (CONFIG_MATROSKA_MUXER && !native_id && > ff_codec_get_tag(ff_codec_movvideo_tags, par->codec_id) && > @@ -2739,7 +2754,12 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv, > ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKGROUP); > ebml_writer_add_block(&writer, mkv); > > - if (duration) > + if (duration > 0 && (par->codec_type == AVMEDIA_TYPE_SUBTITLE || > + /* If the packet's duration is inconsistent with the default duration, > + * add an explicit duration element. */ > + track->default_duration_high > 0 && > + duration != track->default_duration_high && > + duration != track->default_duration_low)) > ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKDURATION, duration); > > av_log(logctx, AV_LOG_DEBUG, > @@ -2917,7 +2937,7 @@ static int mkv_write_packet_internal(AVFormatContext *s, const AVPacket *pkt) > /* All subtitle blocks are considered to be keyframes. */ > int keyframe = is_sub || !!(pkt->flags & AV_PKT_FLAG_KEY); > int64_t duration = FFMAX(pkt->duration, 0); > - int64_t write_duration = is_sub ? duration : 0; > + int64_t cue_duration = is_sub ? duration : 0; > int ret; > int64_t ts = track->write_dts ? pkt->dts : pkt->pts; > int64_t relative_packet_pos; > @@ -2958,7 +2978,7 @@ static int mkv_write_packet_internal(AVFormatContext *s, const AVPacket *pkt) > /* The WebM spec requires WebVTT to be muxed in BlockGroups; > * so we force it even for packets without duration. */ > ret = mkv_write_block(s, mkv, pb, par, track, pkt, > - keyframe, ts, write_duration, > + keyframe, ts, duration, > par->codec_id == AV_CODEC_ID_WEBVTT, > relative_packet_pos); > if (ret < 0) > @@ -2969,7 +2989,7 @@ static int mkv_write_packet_internal(AVFormatContext *s, const AVPacket *pkt) > !mkv->have_video && !track->has_cue)) { > ret = mkv_add_cuepoint(mkv, pkt->stream_index, ts, > mkv->cluster_pos, relative_packet_pos, > - write_duration); > + cue_duration); > if (ret < 0) > return ret; > track->has_cue = 1; > diff --git a/tests/ref/fate/matroska-h264-remux b/tests/ref/fate/matroska-h264-remux > index 2c727f03cd..6362b6f684 100644 > --- a/tests/ref/fate/matroska-h264-remux > +++ b/tests/ref/fate/matroska-h264-remux > @@ -1,5 +1,5 @@ > -f6b943ed3ff05087d0ef58fbaf7abcb0 *tests/data/fate/matroska-h264-remux.matroska > -2036067 tests/data/fate/matroska-h264-remux.matroska > +277a08f708965112a7c2fb25d941e68a *tests/data/fate/matroska-h264-remux.matroska > +2036806 tests/data/fate/matroska-h264-remux.matroska > #tb 0: 1/25 > #media_type 0: video > #codec_id 0: rawvideo _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".