diff mbox series

[FFmpeg-devel,01/20] avformat/matroskaenc: Fix ReferenceBlock timestamp

Message ID 20200101005837.11356-2-andreas.rheinhardt@gmail.com
State Accepted
Headers show
Series Matroska muxer patches
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt Jan. 1, 2020, 12:58 a.m. UTC
In order to indicate that the frames in a BlockGroup are not keyframes,
one has to add a ReferenceBlock element containing the timestamp of a
referenced Block that has already been written. The timestamp ought to be
relative to the timestamp of the Block it is attached to. Yet the
Matroska muxer used the relative timestamp of the preceding Block of the
track, i.e. the timestamp of the preceding block relative to the
timestamp of the Cluster containing said block (that need not be the
Cluster containing the current Block). This has been fixed.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/matroskaenc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andreas Rheinhardt Jan. 7, 2020, 2:12 p.m. UTC | #1
On Wed, Jan 1, 2020 at 1:59 AM Andreas Rheinhardt <
andreas.rheinhardt@gmail.com> wrote:

> In order to indicate that the frames in a BlockGroup are not keyframes,
> one has to add a ReferenceBlock element containing the timestamp of a
> referenced Block that has already been written. The timestamp ought to be
> relative to the timestamp of the Block it is attached to. Yet the
> Matroska muxer used the relative timestamp of the preceding Block of the
> track, i.e. the timestamp of the preceding block relative to the
> timestamp of the Cluster containing said block (that need not be the
> Cluster containing the current Block). This has been fixed.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/matroskaenc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 469b604de6..9cf840c9be 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -2167,9 +2167,9 @@ static void mkv_write_block(AVFormatContext *s,
> AVIOContext *pb,
>          av_free(data);
>
>      if (blockid == MATROSKA_ID_BLOCK && !keyframe) {
> -        put_ebml_sint(pb, MATROSKA_ID_BLOCKREFERENCE,
> track->last_timestamp);
> +        put_ebml_sint(pb, MATROSKA_ID_BLOCKREFERENCE,
> track->last_timestamp - ts);
>      }
> -    track->last_timestamp = ts - mkv->cluster_pts;
> +    track->last_timestamp = ts;
>
>      if (discard_padding) {
>          put_ebml_sint(pb, MATROSKA_ID_DISCARDPADDING, discard_padding);
> --
> 2.20.1
>
>
Ping for the rest of this patchset.

- Andreas
Andreas Rheinhardt Jan. 11, 2020, 2:09 p.m. UTC | #2
On Tue, Jan 7, 2020 at 3:12 PM Andreas Rheinhardt <
andreas.rheinhardt@gmail.com> wrote:

> On Wed, Jan 1, 2020 at 1:59 AM Andreas Rheinhardt <
> andreas.rheinhardt@gmail.com> wrote:
>
>> In order to indicate that the frames in a BlockGroup are not keyframes,
>> one has to add a ReferenceBlock element containing the timestamp of a
>> referenced Block that has already been written. The timestamp ought to be
>> relative to the timestamp of the Block it is attached to. Yet the
>> Matroska muxer used the relative timestamp of the preceding Block of the
>> track, i.e. the timestamp of the preceding block relative to the
>> timestamp of the Cluster containing said block (that need not be the
>> Cluster containing the current Block). This has been fixed.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>>  libavformat/matroskaenc.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 469b604de6..9cf840c9be 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -2167,9 +2167,9 @@ static void mkv_write_block(AVFormatContext *s,
>> AVIOContext *pb,
>>          av_free(data);
>>
>>      if (blockid == MATROSKA_ID_BLOCK && !keyframe) {
>> -        put_ebml_sint(pb, MATROSKA_ID_BLOCKREFERENCE,
>> track->last_timestamp);
>> +        put_ebml_sint(pb, MATROSKA_ID_BLOCKREFERENCE,
>> track->last_timestamp - ts);
>>      }
>> -    track->last_timestamp = ts - mkv->cluster_pts;
>> +    track->last_timestamp = ts;
>>
>>      if (discard_padding) {
>>          put_ebml_sint(pb, MATROSKA_ID_DISCARDPADDING, discard_padding);
>> --
>> 2.20.1
>>
>>
> Ping for the rest of this patchset.
>
> - Andreas
>
>
Ping.

- Andreas
Paul B Mahol Jan. 11, 2020, 2:12 p.m. UTC | #3
probably ok

On 1/1/20, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote:
> In order to indicate that the frames in a BlockGroup are not keyframes,
> one has to add a ReferenceBlock element containing the timestamp of a
> referenced Block that has already been written. The timestamp ought to be
> relative to the timestamp of the Block it is attached to. Yet the
> Matroska muxer used the relative timestamp of the preceding Block of the
> track, i.e. the timestamp of the preceding block relative to the
> timestamp of the Cluster containing said block (that need not be the
> Cluster containing the current Block). This has been fixed.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/matroskaenc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 469b604de6..9cf840c9be 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -2167,9 +2167,9 @@ static void mkv_write_block(AVFormatContext *s,
> AVIOContext *pb,
>          av_free(data);
>
>      if (blockid == MATROSKA_ID_BLOCK && !keyframe) {
> -        put_ebml_sint(pb, MATROSKA_ID_BLOCKREFERENCE,
> track->last_timestamp);
> +        put_ebml_sint(pb, MATROSKA_ID_BLOCKREFERENCE, track->last_timestamp
> - ts);
>      }
> -    track->last_timestamp = ts - mkv->cluster_pts;
> +    track->last_timestamp = ts;
>
>      if (discard_padding) {
>          put_ebml_sint(pb, MATROSKA_ID_DISCARDPADDING, discard_padding);
> --
> 2.20.1
>
> _______________________________________________
> 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".
James Almer Jan. 11, 2020, 2:28 p.m. UTC | #4
On 1/11/2020 11:12 AM, Paul B Mahol wrote:
> probably ok

Applied.

> 
> On 1/1/20, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote:
>> In order to indicate that the frames in a BlockGroup are not keyframes,
>> one has to add a ReferenceBlock element containing the timestamp of a
>> referenced Block that has already been written. The timestamp ought to be
>> relative to the timestamp of the Block it is attached to. Yet the
>> Matroska muxer used the relative timestamp of the preceding Block of the
>> track, i.e. the timestamp of the preceding block relative to the
>> timestamp of the Cluster containing said block (that need not be the
>> Cluster containing the current Block). This has been fixed.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>>  libavformat/matroskaenc.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 469b604de6..9cf840c9be 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -2167,9 +2167,9 @@ static void mkv_write_block(AVFormatContext *s,
>> AVIOContext *pb,
>>          av_free(data);
>>
>>      if (blockid == MATROSKA_ID_BLOCK && !keyframe) {
>> -        put_ebml_sint(pb, MATROSKA_ID_BLOCKREFERENCE,
>> track->last_timestamp);
>> +        put_ebml_sint(pb, MATROSKA_ID_BLOCKREFERENCE, track->last_timestamp
>> - ts);
>>      }
>> -    track->last_timestamp = ts - mkv->cluster_pts;
>> +    track->last_timestamp = ts;
>>
>>      if (discard_padding) {
>>          put_ebml_sint(pb, MATROSKA_ID_DISCARDPADDING, discard_padding);
>> --
>> 2.20.1
diff mbox series

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 469b604de6..9cf840c9be 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -2167,9 +2167,9 @@  static void mkv_write_block(AVFormatContext *s, AVIOContext *pb,
         av_free(data);
 
     if (blockid == MATROSKA_ID_BLOCK && !keyframe) {
-        put_ebml_sint(pb, MATROSKA_ID_BLOCKREFERENCE, track->last_timestamp);
+        put_ebml_sint(pb, MATROSKA_ID_BLOCKREFERENCE, track->last_timestamp - ts);
     }
-    track->last_timestamp = ts - mkv->cluster_pts;
+    track->last_timestamp = ts;
 
     if (discard_padding) {
         put_ebml_sint(pb, MATROSKA_ID_DISCARDPADDING, discard_padding);