Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: fix cue relative position values when CRC32 is enabled

[James Almer]

On 10/25/2016 12:38 PM, James Almer wrote:
> The dynamic buffer does not contain the CRC32 element so calls to avio_tell()
> don't take it into account. This resulted in CueRelativePosition values being
> six bytes short.
> This is a regression since 6724525a1576ca334d2ffdc085620bb44aea7394

Pushed so it makes it to release 3.2, being a fix for a recent regression in a
muxer.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/matroskaenc: fix cue relative position values when CRC32 is enabled

[James Almer]

The dynamic buffer does not contain the CRC32 element so calls to avio_tell()
don't take it into account. This resulted in CueRelativePosition values being
six bytes short.
This is a regression since 6724525a1576ca334d2ffdc085620bb44aea7394

Instead of adding yet another custom check for CRC32 to fix a size or an offset,
remove the existing ones and reserve the six bytes in the dynamic buffer.

Signed-off-by: James Almer 
---
 libavformat/matroskaenc.c | 39 +--
 tests/ref/fate/rgb24-mkv  |  2 +-
 tests/ref/lavf/mkv|  4 ++--
 3 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 03d5326..d91055f 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -322,17 +322,19 @@ static void end_ebml_master(AVIOContext *pb, ebml_master 
master)
 avio_seek(pb, pos, SEEK_SET);
 }
 
-static int start_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, 
ebml_master *master,
-   unsigned int elementid, uint64_t 
expectedsize)
+static int start_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, 
MatroskaMuxContext *mkv,
+   ebml_master *master, unsigned int 
elementid, uint64_t expectedsize)
 {
 int ret;
 
 if ((ret = avio_open_dyn_buf(dyn_cp)) < 0)
 return ret;
 
-if (pb->seekable)
+if (pb->seekable) {
 *master = start_ebml_master(pb, elementid, expectedsize);
-else
+if (mkv->write_crc && mkv->mode != MODE_WEBM)
+put_ebml_void(*dyn_cp, 6); /* Reserve space for CRC32 so 
position/size calculations using avio_tell() take it into account */
+} else
 *master = start_ebml_master(*dyn_cp, elementid, expectedsize);
 
 return 0;
@@ -342,15 +344,16 @@ static void end_ebml_master_crc32(AVIOContext *pb, 
AVIOContext **dyn_cp, Matrosk
   ebml_master master)
 {
 uint8_t *buf, crc[4];
-int size;
+int size, skip = 0;
 
 if (pb->seekable) {
 size = avio_close_dyn_buf(*dyn_cp, );
 if (mkv->write_crc && mkv->mode != MODE_WEBM) {
-AV_WL32(crc, av_crc(av_crc_get_table(AV_CRC_32_IEEE_LE), 
UINT32_MAX, buf, size) ^ UINT32_MAX);
+skip = 6; /* Skip reserved 6-byte long void element from the 
dynamic buffer. */
+AV_WL32(crc, av_crc(av_crc_get_table(AV_CRC_32_IEEE_LE), 
UINT32_MAX, buf + skip, size - skip) ^ UINT32_MAX);
 put_ebml_binary(pb, EBML_ID_CRC32, crc, sizeof(crc));
 }
-avio_write(pb, buf, size);
+avio_write(pb, buf + skip, size - skip);
 end_ebml_master(pb, master);
 } else {
 end_ebml_master(*dyn_cp, master);
@@ -465,7 +468,7 @@ static int64_t mkv_write_seekhead(AVIOContext *pb, 
MatroskaMuxContext *mkv)
 }
 }
 
-if (start_ebml_master_crc32(pb, _cp, , MATROSKA_ID_SEEKHEAD,
+if (start_ebml_master_crc32(pb, _cp, mkv, , 
MATROSKA_ID_SEEKHEAD,
 seekhead->reserved_size) < 0) {
 currentpos = -1;
 goto fail;
@@ -541,7 +544,7 @@ static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues 
*cues, mkv_track *tra
 int i, j, ret;
 
 currentpos = avio_tell(pb);
-ret = start_ebml_master_crc32(pb, _cp, _element, 
MATROSKA_ID_CUES, 0);
+ret = start_ebml_master_crc32(pb, _cp, mkv, _element, 
MATROSKA_ID_CUES, 0);
 if (ret < 0)
 return ret;
 
@@ -1269,7 +1272,7 @@ static int mkv_write_tracks(AVFormatContext *s)
 if (ret < 0)
 return ret;
 
-ret = start_ebml_master_crc32(pb, _cp, , MATROSKA_ID_TRACKS, 0);
+ret = start_ebml_master_crc32(pb, _cp, mkv, , 
MATROSKA_ID_TRACKS, 0);
 if (ret < 0)
 return ret;
 
@@ -1300,7 +1303,7 @@ static int mkv_write_chapters(AVFormatContext *s)
 ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_CHAPTERS, 
avio_tell(pb));
 if (ret < 0) return ret;
 
-ret = start_ebml_master_crc32(pb, _cp, , 
MATROSKA_ID_CHAPTERS, 0);
+ret = start_ebml_master_crc32(pb, _cp, mkv, , 
MATROSKA_ID_CHAPTERS, 0);
 if (ret < 0) return ret;
 
 editionentry = start_ebml_master(dyn_cp, MATROSKA_ID_EDITIONENTRY, 0);
@@ -1387,7 +1390,7 @@ static int mkv_write_tag_targets(AVFormatContext *s,
 ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_TAGS, 
avio_tell(s->pb));
 if (ret < 0) return ret;
 
-start_ebml_master_crc32(s->pb, >tags_bc, tags, MATROSKA_ID_TAGS, 
0);
+start_ebml_master_crc32(s->pb, >tags_bc, mkv, tags, 
MATROSKA_ID_TAGS, 0);
 }
 pb = mkv->tags_bc;
 
@@ -1524,7 +1527,7 @@ static int mkv_write_tags(AVFormatContext *s)
 
 if (mkv->tags.pos) {
 if (s->pb->seekable && !mkv->is_live)
-put_ebml_void(s->pb, avio_tell(mkv->tags_bc) + ((mkv->write_crc && 
mkv->mode != MODE_WEBM) ? 2 /* ebml id + data size */ + 4 /* CRC32 */ : 0));
+put_ebml_void(s->pb,