From db6b2ca0b3afe898e8c15176f57b85fdf27cede1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= Date: Fri, 28 Feb 2014 11:41:55 +0200 Subject: [PATCH 1/8] qt-faststart: Increase the copy buffer size to 64 KB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copying data in chunks of 1 KB is a little wasteful. 64 KB should still easily fit on the stack, so there's no need to allocate it dynamically. Signed-off-by: Martin Storsjö (cherry picked from commit 3cbc7ef3d60b6af3617079f24a4f401d83353003) --- tools/qt-faststart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/qt-faststart.c b/tools/qt-faststart.c index f33d6fa80c..88f5b48785 100644 --- a/tools/qt-faststart.c +++ b/tools/qt-faststart.c @@ -77,7 +77,7 @@ #define CO64_ATOM QT_ATOM('c', 'o', '6', '4') #define ATOM_PREAMBLE_SIZE 8 -#define COPY_BUFFER_SIZE 1024 +#define COPY_BUFFER_SIZE 65536 int main(int argc, char *argv[]) { From c9f015f1c6887ffcbfef119ea1e4846b800fdfff Mon Sep 17 00:00:00 2001 From: Michael Niedermayer Date: Sun, 30 Sep 2012 21:53:26 +0200 Subject: [PATCH 2/8] qt-faststart: Simplify code by using a MIN() macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qt-faststart doesn't use the normal libav headers at all since it's supposed to be a completely standalone tool, so we implement the macro locally in this file. Signed-off-by: Martin Storsjö (cherry picked from commit ea15a9a5d8fa6a71af3101b2af18c4dcac07987f) --- tools/qt-faststart.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/qt-faststart.c b/tools/qt-faststart.c index 88f5b48785..69818be2da 100644 --- a/tools/qt-faststart.c +++ b/tools/qt-faststart.c @@ -37,6 +37,8 @@ #define ftello(x) _ftelli64(x) #endif +#define MIN(a,b) ((a) > (b) ? (b) : (a)) + #define BE_16(x) ((((uint8_t*)(x))[0] << 8) | ((uint8_t*)(x))[1]) #define BE_32(x) ((((uint8_t*)(x))[0] << 24) | \ @@ -295,10 +297,7 @@ int main(int argc, char *argv[]) /* copy the remainder of the infile, from offset 0 -> last_offset - 1 */ printf(" copying rest of file...\n"); while (last_offset) { - if (last_offset > COPY_BUFFER_SIZE) - bytes_to_copy = COPY_BUFFER_SIZE; - else - bytes_to_copy = last_offset; + bytes_to_copy = MIN(COPY_BUFFER_SIZE, last_offset); if (fread(copy_buffer, bytes_to_copy, 1, infile) != 1) { perror(argv[1]); From 92edc13d69188de1f12197996334c1cc6e2d12c5 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer Date: Mon, 22 Oct 2012 22:42:51 +0200 Subject: [PATCH 3/8] qt-faststart: Check fseeko() return codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Storsjö (cherry picked from commit 5612244351b2eb3cb4e6225861a0f55aa5d0c475) --- tools/qt-faststart.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/tools/qt-faststart.c b/tools/qt-faststart.c index 69818be2da..0abcd2b67c 100644 --- a/tools/qt-faststart.c +++ b/tools/qt-faststart.c @@ -136,22 +136,27 @@ int main(int argc, char *argv[]) atom_size); goto error_out; } - fseeko(infile, -ATOM_PREAMBLE_SIZE, SEEK_CUR); - if (fread(ftyp_atom, atom_size, 1, infile) != 1) { + if (fseeko(infile, -ATOM_PREAMBLE_SIZE, SEEK_CUR) || + fread(ftyp_atom, atom_size, 1, infile) != 1) { perror(argv[1]); goto error_out; } start_offset = ftello(infile); } else { + int ret; /* 64-bit special case */ if (atom_size == 1) { if (fread(atom_bytes, ATOM_PREAMBLE_SIZE, 1, infile) != 1) { break; } atom_size = BE_64(&atom_bytes[0]); - fseeko(infile, atom_size - ATOM_PREAMBLE_SIZE * 2, SEEK_CUR); + ret = fseeko(infile, atom_size - ATOM_PREAMBLE_SIZE * 2, SEEK_CUR); } else { - fseeko(infile, atom_size - ATOM_PREAMBLE_SIZE, SEEK_CUR); + ret = fseeko(infile, atom_size - ATOM_PREAMBLE_SIZE, SEEK_CUR); + } + if (ret) { + perror(argv[1]); + goto error_out; } } printf("%c%c%c%c %10"PRIu64" %"PRIu64"\n", @@ -192,7 +197,10 @@ int main(int argc, char *argv[]) /* moov atom was, in fact, the last atom in the chunk; load the whole * moov atom */ - fseeko(infile, -atom_size, SEEK_END); + if (fseeko(infile, -atom_size, SEEK_END)) { + perror(argv[1]); + goto error_out; + } last_offset = ftello(infile); moov_atom_size = atom_size; moov_atom = malloc(moov_atom_size); @@ -268,7 +276,11 @@ int main(int argc, char *argv[]) } if (start_offset > 0) { /* seek after ftyp atom */ - fseeko(infile, start_offset, SEEK_SET); + if (fseeko(infile, start_offset, SEEK_SET)) { + perror(argv[1]); + goto error_out; + } + last_offset -= start_offset; } From 298d66c8de1d4faa152f4ef557566aa0519223d8 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer Date: Mon, 29 Oct 2012 22:05:33 +0100 Subject: [PATCH 4/8] qt-faststart: Fix the signedness of variables keeping the ftello return values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These variables are assigned the return values of ftello, which returns an off_t, which is a signed type. On errors, ftello returns -1, thus make sure this error return value can be stored properly. Signed-off-by: Martin Storsjö (cherry picked from commit 03c2a66fcff9707f71ffef7e61ce5e3973220d4b) --- tools/qt-faststart.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/qt-faststart.c b/tools/qt-faststart.c index 0abcd2b67c..2b2e00ca57 100644 --- a/tools/qt-faststart.c +++ b/tools/qt-faststart.c @@ -89,7 +89,7 @@ int main(int argc, char *argv[]) uint32_t atom_type = 0; uint64_t atom_size = 0; uint64_t atom_offset = 0; - uint64_t last_offset; + int64_t last_offset; unsigned char *moov_atom = NULL; unsigned char *ftyp_atom = NULL; uint64_t moov_atom_size; @@ -97,7 +97,7 @@ int main(int argc, char *argv[]) uint64_t i, j; uint32_t offset_count; uint64_t current_offset; - uint64_t start_offset = 0; + int64_t start_offset = 0; unsigned char copy_buffer[COPY_BUFFER_SIZE]; int bytes_to_copy; From b3f106cb1f0036ce54ead5b59120fed7d7aa11d7 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer Date: Thu, 25 Oct 2012 00:39:33 +0200 Subject: [PATCH 5/8] qt-faststart: Check the ftello() return codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This silences a warning in the coverity static analyzer. Signed-off-by: Martin Storsjö (cherry picked from commit 63848854256a024a19435e87d6bc76fffa65e81e) --- tools/qt-faststart.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/qt-faststart.c b/tools/qt-faststart.c index 2b2e00ca57..5c511a0154 100644 --- a/tools/qt-faststart.c +++ b/tools/qt-faststart.c @@ -137,11 +137,11 @@ int main(int argc, char *argv[]) goto error_out; } if (fseeko(infile, -ATOM_PREAMBLE_SIZE, SEEK_CUR) || - fread(ftyp_atom, atom_size, 1, infile) != 1) { + fread(ftyp_atom, atom_size, 1, infile) != 1 || + (start_offset = ftello(infile)) < 0) { perror(argv[1]); goto error_out; } - start_offset = ftello(infile); } else { int ret; /* 64-bit special case */ @@ -202,6 +202,10 @@ int main(int argc, char *argv[]) goto error_out; } last_offset = ftello(infile); + if (last_offset < 0) { + perror(argv[1]); + goto error_out; + } moov_atom_size = atom_size; moov_atom = malloc(moov_atom_size); if (!moov_atom) { From 7754d4838178a5c09c3c3953bb2b90d1abc639e3 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer Date: Thu, 13 Dec 2012 15:07:20 +0100 Subject: [PATCH 6/8] qt-faststart: Check offset_count before reading from the moov_atom buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CC: libav-stable@libav.org Signed-off-by: Martin Storsjö (cherry picked from commit bb95334c34d0d9abccea370ae25c4765d7764ab8) --- tools/qt-faststart.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/qt-faststart.c b/tools/qt-faststart.c index 5c511a0154..792c272193 100644 --- a/tools/qt-faststart.c +++ b/tools/qt-faststart.c @@ -239,6 +239,10 @@ int main(int argc, char *argv[]) goto error_out; } offset_count = BE_32(&moov_atom[i + 8]); + if (i + 12 + offset_count * UINT64_C(4) > moov_atom_size) { + printf(" bad atom size/element count\n"); + goto error_out; + } for (j = 0; j < offset_count; j++) { current_offset = BE_32(&moov_atom[i + 12 + j * 4]); current_offset += moov_atom_size; @@ -256,6 +260,10 @@ int main(int argc, char *argv[]) goto error_out; } offset_count = BE_32(&moov_atom[i + 8]); + if (i + 12 + offset_count * UINT64_C(8) > moov_atom_size) { + printf(" bad atom size/element count\n"); + goto error_out; + } for (j = 0; j < offset_count; j++) { current_offset = BE_64(&moov_atom[i + 12 + j * 8]); current_offset += moov_atom_size; From 9841617b7f862fcf24ad05eda865a3f323ee0dd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= Date: Fri, 28 Feb 2014 12:19:49 +0200 Subject: [PATCH 7/8] qt-faststart: Avoid unintentionally sign extending BE_32 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without this cast, the BE_32() expression is sign extended when assigned to an uint64_t, since the uint8_t|uint8_t expression is promoted to an int. Also avoid undefined behaviour when left shifting an uint8_t by 24 by casting it to an uint32_t explicitly before shifting. Based on a patch by Michael Niedermayer. Signed-off-by: Martin Storsjö (cherry picked from commit ea7f79f93796d68559a495be824b6bbd94dfe5f6) --- tools/qt-faststart.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/qt-faststart.c b/tools/qt-faststart.c index 792c272193..3a0139fecb 100644 --- a/tools/qt-faststart.c +++ b/tools/qt-faststart.c @@ -41,10 +41,10 @@ #define BE_16(x) ((((uint8_t*)(x))[0] << 8) | ((uint8_t*)(x))[1]) -#define BE_32(x) ((((uint8_t*)(x))[0] << 24) | \ - (((uint8_t*)(x))[1] << 16) | \ - (((uint8_t*)(x))[2] << 8) | \ - ((uint8_t*)(x))[3]) +#define BE_32(x) (((uint32_t)(((uint8_t*)(x))[0]) << 24) | \ + (((uint8_t*)(x))[1] << 16) | \ + (((uint8_t*)(x))[2] << 8) | \ + ((uint8_t*)(x))[3]) #define BE_64(x) (((uint64_t)(((uint8_t*)(x))[0]) << 56) | \ ((uint64_t)(((uint8_t*)(x))[1]) << 48) | \ @@ -123,7 +123,7 @@ int main(int argc, char *argv[]) if (fread(atom_bytes, ATOM_PREAMBLE_SIZE, 1, infile) != 1) { break; } - atom_size = (uint32_t) BE_32(&atom_bytes[0]); + atom_size = BE_32(&atom_bytes[0]); atom_type = BE_32(&atom_bytes[4]); /* keep ftyp atom */ From a6a2d8eb8f125a2edb512a7a47df33dbd70d6b35 Mon Sep 17 00:00:00 2001 From: Lou Logan Date: Tue, 7 Jan 2014 10:59:04 -0900 Subject: [PATCH 8/8] qt-faststart: Add a note about the -movflags +faststart feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Storsjö (cherry picked from commit 700687ebe07ac8b7a5cf5fd2c4892ea07a827080) --- tools/qt-faststart.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/qt-faststart.c b/tools/qt-faststart.c index 3a0139fecb..ed6de1b86c 100644 --- a/tools/qt-faststart.c +++ b/tools/qt-faststart.c @@ -102,7 +102,8 @@ int main(int argc, char *argv[]) int bytes_to_copy; if (argc != 3) { - printf("Usage: qt-faststart \n"); + printf("Usage: qt-faststart \n" + "Note: alternatively you can use -movflags +faststart in avconv\n"); return 0; }