On 03/21/2015 10:28 AM, John Stebbins wrote: > On 03/21/2015 09:45 AM, wm4 wrote: >> On Sat, 21 Mar 2015 09:20:40 -0600 >> John Stebbins <[email protected]> wrote: >> >>> On 03/21/2015 06:30 AM, wm4 wrote: >>>> On Fri, 20 Mar 2015 16:47:57 -0600 >>>> John Stebbins <[email protected]> wrote: >>>> >>>>> Some subtitles (e.g. PGS) do not require a duration since they have >>>>> explicit end-of-subtitle indication in the stream. This provides >>>>> a way to omit the unnecessary duration while muxing. >>>>> --- >>>>> libavformat/matroskaenc.c | 3 ++- >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c >>>>> index 49e5bf3..1dd8d74 100644 >>>>> --- a/libavformat/matroskaenc.c >>>>> +++ b/libavformat/matroskaenc.c >>>>> @@ -1524,7 +1524,8 @@ static int >>>>> mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt) >>>>> >>>>> mkv_blockgroup_size(pkt->size)); >>>>> duration = pkt->convergence_duration; >>>>> mkv_write_block(s, pb, MATROSKA_ID_BLOCK, pkt, 0); >>>>> - put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, duration); >>>>> + if (pkt->convergence_duration != AV_NOPTS_VALUE) >>>>> + put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, duration); >>>>> end_ebml_master(pb, blockgroup); >>>>> } >>>>> >>>> It's weird that it checks convergence_duration when writing duration. I >>>> also wasn't aware that convergence_duration can be set to AV_NOPTS_VALUE >>>> to signal that it's unset. Isn't 0 the value for unset duration? (The >>>> API can't distinguish between 0 duration and unset duration, this is a >>>> known problem.) >>>> >>> If you recall, a while back I proposed a patch that used 0 for unset. I'm >>> just (finally) following up on that thread. >>> Your response was this: >>> https://lists.libav.org/pipermail/libav-devel/2014-October/064418.html >>> >>> You didn't think using 0 for this was a good idea because even mkvmerge >>> wrote 0 for block duration despite the >>> discrepancy with the "matroska spec". As for AV_NOPTS_VALUE and >>> convergence_duration, avcodec.h says "Is AV_NOPTS_VALUE >>> if unknown" >>> >>> >> In that older post I meant that 0 should not be written to the >> file. If convergence_duration==AV_NOPTS_VALUE is used consistently it's >> fine, although avpacket.c still initializes it to 0 by default. >> >> (Also, isn't it time to break ABI and unify duration and >> convergence_duration?) >> > Using either 0 or AV_NOPTS_VALUE appears to be safe. Either is fine by me. > So if there is a preference, speak up. > A quick grep shows that convergence_duration is only used in matroskaenc and > matroskadec. It wouldn't be > hard to drop it in favor of duration. But that's a change you wouldn't want > backported to v11 and I would > like this change in behavior to get into v11 if possible. >
No additional opinions? IMO each option has it's own advantages. Using AV_NOPTS_VALUE maintains current functionality when convergence_duration == 0 which may prevent breakage in some scenario that expects this behavior. On the other hand, preventing 0 BLOCKDURATION adheres to the letter of the matroska "spec". I prefer the latter (which was my original patch), so if there are no objections, I will re-submit that patch given that I seem to have misinterpreted wm4's comments regarding it. -- John GnuPG fingerprint: D0EC B3DB C372 D1F1 0B01 83F0 49F1 D7B2 60D4 D0F7
signature.asc
Description: OpenPGP digital signature
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
