| Message ID | CAB0OVGpap1t0N-Ov+BSPN3y71NZP8xqNY79BjQhUkptSp1RTeA@mail.gmail.com |
|---|---|
| State | Withdrawn |
| Headers | show |
On 12/11/2017 03:12, Carl Eugen Hoyos wrote: > - put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, duration); > + if (duration > 0) > + put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, duration); In that case, the duration of the block is DefaultDuration (if it exists), completely different. Is it intended to say that a duration of 0 at this place means that the block has the default block duration if it exists? What is the use case for a duration of 0? (Note: the meaning of the a block duration of 0 is unclear to me, I'll ask on CELLAR mailing list to clarify that)
2017-11-14 19:02 GMT+01:00 Jerome Martinez <jerome@mediaarea.net>: > On 12/11/2017 03:12, Carl Eugen Hoyos wrote: >> >> - put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, duration); >> + if (duration > 0) >> + put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, duration); > > > In that case, the duration of the block is DefaultDuration (if it exists), > completely different. > Is it intended to say that a duration of 0 at this place means that the > block has the default block duration if it exists? > > What is the use case for a duration of 0? Iirc, pgs subtitles send an empty "frame" to reset / remove the subtitle. > (Note: the meaning of the a block duration of 0 is unclear to me, I'll ask > on CELLAR mailing list to clarify that) So maybe the issue is with mkvalidate? Carl Eugen
On 11/14/2017 02:18 PM, Carl Eugen Hoyos wrote: > 2017-11-14 19:02 GMT+01:00 Jerome Martinez <jerome@mediaarea.net>: >> On 12/11/2017 03:12, Carl Eugen Hoyos wrote: >>> - put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, duration); >>> + if (duration > 0) >>> + put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, duration); >> >> In that case, the duration of the block is DefaultDuration (if it exists), >> completely different. >> Is it intended to say that a duration of 0 at this place means that the >> block has the default block duration if it exists? >> >> What is the use case for a duration of 0? > Iirc, pgs subtitles send an empty "frame" to reset / remove the > subtitle. Correct, this is the source of these 0 duration pgs "subtitles". > >> (Note: the meaning of the a block duration of 0 is unclear to me, I'll ask >> on CELLAR mailing list to clarify that) > So maybe the issue is with mkvalidate? > > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 12/11/2017 03:12, Carl Eugen Hoyos wrote: > The matroska spec says blockduration == 0 means the frame is not a > keyframe. Since all subtitles are "keyframes", 0 blockduration should > not be written. The issue is in the specifications: https://github.com/Matroska-Org/matroska-specification/pull/207 The patch should not be integrated as the reason is no more valid.
2017-11-16 9:07 GMT+01:00 Jerome Martinez <jerome@mediaarea.net>: > On 12/11/2017 03:12, Carl Eugen Hoyos wrote: >> >> The matroska spec says blockduration == 0 means the frame is not a >> keyframe. Since all subtitles are "keyframes", 0 blockduration should >> not be written. > > > The issue is in the specifications: > https://github.com/Matroska-Org/matroska-specification/pull/207 > > The patch should not be integrated as the reason is no more valid. Thank you! Carl Eugen
On 12/11/2017 03:12, Carl Eugen Hoyos wrote: > The matroska spec says blockduration == 0 means the frame is not a > keyframe. Since all subtitles are "keyframes", 0 blockduration should > not be written. As I understand from discussion on CELLAR mailing-list: - if is not expected to have a frame with block duration of 0, in a perfect world the previous frame should have the right duration and the player should hide the previous frame after blockduration of the previous frame. - as a workaround, it would make sense to put an empty frame with "unlimited" duration, so no blockduration, as implemented in the patch. The only issue is in the case someone puts a "not empty" frame with a duration of 0, this patch would change the behavior (from nothing displayed to the frame displayed) but as it is not expected to have a duration of 0 when something is displayed, this is not a real use case IMO. Suggestion of changes for this patch: - Remove the outdated part about specs. A duration of 0 is not coherent, that's all. - Add a comment (in the code?) stipulating that duration of 0 is a special case which means "up to the next frame".
On 11/23/2017 03:04 AM, Jerome Martinez wrote: > On 12/11/2017 03:12, Carl Eugen Hoyos wrote: >> The matroska spec says blockduration == 0 means the frame is not a >> keyframe. Since all subtitles are "keyframes", 0 blockduration should >> not be written. > As I understand from discussion on CELLAR mailing-list: > - if is not expected to have a frame with block duration of 0, in a > perfect world the previous frame should have the right duration and the > player should hide the previous frame after blockduration of the > previous frame. > - as a workaround, it would make sense to put an empty frame with > "unlimited" duration, so no blockduration, as implemented in the patch. > > The only issue is in the case someone puts a "not empty" frame with a > duration of 0, this patch would change the behavior (from nothing > displayed to the frame displayed) but as it is not expected to have a > duration of 0 when something is displayed, this is not a real use case IMO. > > Suggestion of changes for this patch: > - Remove the outdated part about specs. A duration of 0 is not coherent, > that's all. > - Add a comment (in the code?) stipulating that duration of 0 is a > special case which means "up to the next frame". > > The use case this patch was created for is PGS subtitles that use an "empty" PGS subtitle to mark the end of the previous subtitle. An "empty" PGS subtitle is not a packet of 0 length. It is a PGS subtitle that has zero composition objects. So a non-empty frame of 0 duration gets written to mark the end of the previous PGS. It's not practical to know the duration of the previous subtitle before writing it because you can't know it till you have seen this empty PGS. Once you've seen the empty PGS, it is often too late to properly interleave the previous PGS in the output file. Your suggestion (0 means up to next frame) however works I believe. I.e. display the "empty" PGS up to the next frame.
On 23/11/2017 20:02, John Stebbins wrote: > [...] It's not practical to > know the duration of the previous subtitle before writing it because you can't know it till you have seen this empty > PGS. Once you've seen the empty PGS, it is often too late to properly interleave the previous PGS in the output file. I argued that on CELLAR mailing list (this is especially true in the case of live streaming) but did not find consensus about the need to have a different semantic for "clear" compared to "display empty subtitle", so for "clear" you have to use "display empty subtitle" thing as a workaround. It may change in the future. > > Your suggestion (0 means up to next frame) however works I believe. I.e. display the "empty" PGS up to the next frame. Right. I actually don't change the code in the patch, just the reason.
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index dad6d6c..61ce0f7 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -2396,7 +2396,8 @@ FF_ENABLE_DEPRECATION_WARNINGS #endif /* All subtitle blocks are considered to be keyframes. */ mkv_write_block(s, pb, MATROSKA_ID_BLOCK, pkt, 1); - put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, duration); + if (duration > 0) + put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, duration); end_ebml_master(pb, blockgroup); }
Hi! Attached patch by John Stebbins fixes a warning with mkvalidator here. Please comment, Carl Eugen From dab79ea5cd01187567b1761aaf1c329926483786 Wed Oct 29 00:00:00 2014 From: John Stebbins <stebbins@jetheaddev.com> Date: Wed, 29 Oct 2014 10:54:44 -0700 Subject: [PATCH] lavf/matroskaenc: Fix writing zero duration subtitles The matroska spec says blockduration == 0 means the frame is not a keyframe. Since all subtitles are "keyframes", 0 blockduration should not be written. Fixes mkvalidator error messages for PGS subtitles. --- libavformat/matroskaenc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)