diff mbox series

[FFmpeg-devel] avformat/mov: make STTS duration unsigned int

Message ID 20211116141533.1708-1-ffmpeg@gyani.pro
State Accepted
Commit 203b0e3561dea1ec459be226d805abe73e7535e5
Headers show
Series [FFmpeg-devel] avformat/mov: make STTS duration unsigned int
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/makex86 warning New warnings during build
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished
andriy/makeppc warning New warnings during build

Commit Message

Gyan Doshi Nov. 16, 2021, 2:15 p.m. UTC
As per 8.6.1.2.2 of ISO/IEC 14496-12:2015(E), STTS sample offsets
are to be always stored as uint32_t. So far, they have been signed ints
which led to desync in files with very large offsets.

The MOVStts struct was used to store CTTS offsets as well. These can be
negative in version 1. So a new struct MOVCtts was created and all
declarations for CTTS usage changed to MOVCtts.
---
The muxer should be adjusted to be able to write larger durations in the
stts but that's a larger undertaking which I'll do later.

 libavformat/isom.h   |  9 +++++++--
 libavformat/mov.c    | 20 ++++++++++----------
 libavformat/movenc.c |  2 +-
 3 files changed, 18 insertions(+), 13 deletions(-)

Comments

Gyan Doshi Nov. 19, 2021, 4:02 p.m. UTC | #1
Plan to push Monday afternoon.

On 2021-11-16 07:45 pm, Gyan Doshi wrote:
> As per 8.6.1.2.2 of ISO/IEC 14496-12:2015(E), STTS sample offsets
> are to be always stored as uint32_t. So far, they have been signed ints
> which led to desync in files with very large offsets.
>
> The MOVStts struct was used to store CTTS offsets as well. These can be
> negative in version 1. So a new struct MOVCtts was created and all
> declarations for CTTS usage changed to MOVCtts.
> ---
> The muxer should be adjusted to be able to write larger durations in the
> stts but that's a larger undertaking which I'll do later.
>
>   libavformat/isom.h   |  9 +++++++--
>   libavformat/mov.c    | 20 ++++++++++----------
>   libavformat/movenc.c |  2 +-
>   3 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index f3c18c95be..ef8f19b18c 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -55,9 +55,14 @@ struct AVAESCTR;
>   
>   typedef struct MOVStts {
>       unsigned int count;
> -    int duration;
> +    unsigned int duration;
>   } MOVStts;
>   
> +typedef struct MOVCtts {
> +    unsigned int count;
> +    int duration;
> +} MOVCtts;
> +
>   typedef struct MOVStsc {
>       int first;
>       int count;
> @@ -168,7 +173,7 @@ typedef struct MOVStreamContext {
>       uint8_t *sdtp_data;
>       unsigned int ctts_count;
>       unsigned int ctts_allocated_size;
> -    MOVStts *ctts_data;
> +    MOVCtts *ctts_data;
>       unsigned int stsc_count;
>       MOVStsc *stsc_data;
>       unsigned int stsc_index;
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 8a910a3165..451cb78bbf 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -77,7 +77,7 @@ typedef struct MOVParseTableEntry {
>   
>   static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom);
>   static int mov_read_mfra(MOVContext *c, AVIOContext *f);
> -static int64_t add_ctts_entry(MOVStts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size,
> +static int64_t add_ctts_entry(MOVCtts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size,
>                                 int count, int duration);
>   
>   static int mov_metadata_track_or_disc_number(MOVContext *c, AVIOContext *pb,
> @@ -2938,7 +2938,7 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>           return AVERROR(ENOMEM);
>   
>       for (i = 0; i < entries && !pb->eof_reached; i++) {
> -        int sample_duration;
> +        unsigned int sample_duration;
>           unsigned int sample_count;
>           unsigned int min_entries = FFMIN(FFMAX(i + 1, 1024 * 1024), entries);
>           MOVStts *stts_data = av_fast_realloc(sc->stts_data, &alloc_size,
> @@ -3191,7 +3191,7 @@ static int get_edit_list_entry(MOVContext *mov,
>   static int find_prev_closest_index(AVStream *st,
>                                      AVIndexEntry *e_old,
>                                      int nb_old,
> -                                   MOVStts* ctts_data,
> +                                   MOVCtts* ctts_data,
>                                      int64_t ctts_count,
>                                      int64_t timestamp_pts,
>                                      int flag,
> @@ -3342,17 +3342,17 @@ static void fix_index_entry_timestamps(AVStream* st, int end_index, int64_t end_
>    * Append a new ctts entry to ctts_data.
>    * Returns the new ctts_count if successful, else returns -1.
>    */
> -static int64_t add_ctts_entry(MOVStts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size,
> +static int64_t add_ctts_entry(MOVCtts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size,
>                                 int count, int duration)
>   {
> -    MOVStts *ctts_buf_new;
> -    const size_t min_size_needed = (*ctts_count + 1) * sizeof(MOVStts);
> +    MOVCtts *ctts_buf_new;
> +    const size_t min_size_needed = (*ctts_count + 1) * sizeof(MOVCtts);
>       const size_t requested_size =
>           min_size_needed > *allocated_size ?
>           FFMAX(min_size_needed, 2 * (*allocated_size)) :
>           min_size_needed;
>   
> -    if ((unsigned)(*ctts_count) >= UINT_MAX / sizeof(MOVStts) - 1)
> +    if ((unsigned)(*ctts_count) >= UINT_MAX / sizeof(MOVCtts) - 1)
>           return -1;
>   
>       ctts_buf_new = av_fast_realloc(*ctts_data, allocated_size, requested_size);
> @@ -3486,7 +3486,7 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
>       int nb_old = sti->nb_index_entries;
>       const AVIndexEntry *e_old_end = e_old + nb_old;
>       const AVIndexEntry *current = NULL;
> -    MOVStts *ctts_data_old = msc->ctts_data;
> +    MOVCtts *ctts_data_old = msc->ctts_data;
>       int64_t ctts_index_old = 0;
>       int64_t ctts_sample_old = 0;
>       int64_t ctts_count_old = msc->ctts_count;
> @@ -3793,7 +3793,7 @@ static void mov_build_index(MOVContext *mov, AVStream *st)
>       unsigned int stps_index = 0;
>       unsigned int i, j;
>       uint64_t stream_size = 0;
> -    MOVStts *ctts_data_old = sc->ctts_data;
> +    MOVCtts *ctts_data_old = sc->ctts_data;
>       unsigned int ctts_count_old = sc->ctts_count;
>   
>       if (sc->elst_count) {
> @@ -4754,7 +4754,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>       AVStream *st = NULL;
>       FFStream *sti = NULL;
>       MOVStreamContext *sc;
> -    MOVStts *ctts_data;
> +    MOVCtts *ctts_data;
>       uint64_t offset;
>       int64_t dts, pts = AV_NOPTS_VALUE;
>       int data_offset = 0;
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 37d4403f7a..233aee62d4 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -2434,7 +2434,7 @@ static int mov_write_stsd_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContext
>   static int mov_write_ctts_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track)
>   {
>       MOVMuxContext *mov = s->priv_data;
> -    MOVStts *ctts_entries;
> +    MOVCtts *ctts_entries;
>       uint32_t entries = 0;
>       uint32_t atom_size;
>       int i;
Gyan Doshi Nov. 22, 2021, 9:49 a.m. UTC | #2
On 2021-11-19 09:32 pm, Gyan Doshi wrote:
> Plan to push Monday afternoon.

Pushed as 203b0e3561dea1ec459be226d805abe73e7535e5

Regards,
Gyan


>
> On 2021-11-16 07:45 pm, Gyan Doshi wrote:
>> As per 8.6.1.2.2 of ISO/IEC 14496-12:2015(E), STTS sample offsets
>> are to be always stored as uint32_t. So far, they have been signed ints
>> which led to desync in files with very large offsets.
>>
>> The MOVStts struct was used to store CTTS offsets as well. These can be
>> negative in version 1. So a new struct MOVCtts was created and all
>> declarations for CTTS usage changed to MOVCtts.
>> ---
>> The muxer should be adjusted to be able to write larger durations in the
>> stts but that's a larger undertaking which I'll do later.
>>
>>   libavformat/isom.h   |  9 +++++++--
>>   libavformat/mov.c    | 20 ++++++++++----------
>>   libavformat/movenc.c |  2 +-
>>   3 files changed, 18 insertions(+), 13 deletions(-)
>>
>> diff --git a/libavformat/isom.h b/libavformat/isom.h
>> index f3c18c95be..ef8f19b18c 100644
>> --- a/libavformat/isom.h
>> +++ b/libavformat/isom.h
>> @@ -55,9 +55,14 @@ struct AVAESCTR;
>>     typedef struct MOVStts {
>>       unsigned int count;
>> -    int duration;
>> +    unsigned int duration;
>>   } MOVStts;
>>   +typedef struct MOVCtts {
>> +    unsigned int count;
>> +    int duration;
>> +} MOVCtts;
>> +
>>   typedef struct MOVStsc {
>>       int first;
>>       int count;
>> @@ -168,7 +173,7 @@ typedef struct MOVStreamContext {
>>       uint8_t *sdtp_data;
>>       unsigned int ctts_count;
>>       unsigned int ctts_allocated_size;
>> -    MOVStts *ctts_data;
>> +    MOVCtts *ctts_data;
>>       unsigned int stsc_count;
>>       MOVStsc *stsc_data;
>>       unsigned int stsc_index;
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 8a910a3165..451cb78bbf 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -77,7 +77,7 @@ typedef struct MOVParseTableEntry {
>>     static int mov_read_default(MOVContext *c, AVIOContext *pb, 
>> MOVAtom atom);
>>   static int mov_read_mfra(MOVContext *c, AVIOContext *f);
>> -static int64_t add_ctts_entry(MOVStts** ctts_data, unsigned int* 
>> ctts_count, unsigned int* allocated_size,
>> +static int64_t add_ctts_entry(MOVCtts** ctts_data, unsigned int* 
>> ctts_count, unsigned int* allocated_size,
>>                                 int count, int duration);
>>     static int mov_metadata_track_or_disc_number(MOVContext *c, 
>> AVIOContext *pb,
>> @@ -2938,7 +2938,7 @@ static int mov_read_stts(MOVContext *c, 
>> AVIOContext *pb, MOVAtom atom)
>>           return AVERROR(ENOMEM);
>>         for (i = 0; i < entries && !pb->eof_reached; i++) {
>> -        int sample_duration;
>> +        unsigned int sample_duration;
>>           unsigned int sample_count;
>>           unsigned int min_entries = FFMIN(FFMAX(i + 1, 1024 * 1024), 
>> entries);
>>           MOVStts *stts_data = av_fast_realloc(sc->stts_data, 
>> &alloc_size,
>> @@ -3191,7 +3191,7 @@ static int get_edit_list_entry(MOVContext *mov,
>>   static int find_prev_closest_index(AVStream *st,
>>                                      AVIndexEntry *e_old,
>>                                      int nb_old,
>> -                                   MOVStts* ctts_data,
>> +                                   MOVCtts* ctts_data,
>>                                      int64_t ctts_count,
>>                                      int64_t timestamp_pts,
>>                                      int flag,
>> @@ -3342,17 +3342,17 @@ static void 
>> fix_index_entry_timestamps(AVStream* st, int end_index, int64_t end_
>>    * Append a new ctts entry to ctts_data.
>>    * Returns the new ctts_count if successful, else returns -1.
>>    */
>> -static int64_t add_ctts_entry(MOVStts** ctts_data, unsigned int* 
>> ctts_count, unsigned int* allocated_size,
>> +static int64_t add_ctts_entry(MOVCtts** ctts_data, unsigned int* 
>> ctts_count, unsigned int* allocated_size,
>>                                 int count, int duration)
>>   {
>> -    MOVStts *ctts_buf_new;
>> -    const size_t min_size_needed = (*ctts_count + 1) * sizeof(MOVStts);
>> +    MOVCtts *ctts_buf_new;
>> +    const size_t min_size_needed = (*ctts_count + 1) * sizeof(MOVCtts);
>>       const size_t requested_size =
>>           min_size_needed > *allocated_size ?
>>           FFMAX(min_size_needed, 2 * (*allocated_size)) :
>>           min_size_needed;
>>   -    if ((unsigned)(*ctts_count) >= UINT_MAX / sizeof(MOVStts) - 1)
>> +    if ((unsigned)(*ctts_count) >= UINT_MAX / sizeof(MOVCtts) - 1)
>>           return -1;
>>         ctts_buf_new = av_fast_realloc(*ctts_data, allocated_size, 
>> requested_size);
>> @@ -3486,7 +3486,7 @@ static void mov_fix_index(MOVContext *mov, 
>> AVStream *st)
>>       int nb_old = sti->nb_index_entries;
>>       const AVIndexEntry *e_old_end = e_old + nb_old;
>>       const AVIndexEntry *current = NULL;
>> -    MOVStts *ctts_data_old = msc->ctts_data;
>> +    MOVCtts *ctts_data_old = msc->ctts_data;
>>       int64_t ctts_index_old = 0;
>>       int64_t ctts_sample_old = 0;
>>       int64_t ctts_count_old = msc->ctts_count;
>> @@ -3793,7 +3793,7 @@ static void mov_build_index(MOVContext *mov, 
>> AVStream *st)
>>       unsigned int stps_index = 0;
>>       unsigned int i, j;
>>       uint64_t stream_size = 0;
>> -    MOVStts *ctts_data_old = sc->ctts_data;
>> +    MOVCtts *ctts_data_old = sc->ctts_data;
>>       unsigned int ctts_count_old = sc->ctts_count;
>>         if (sc->elst_count) {
>> @@ -4754,7 +4754,7 @@ static int mov_read_trun(MOVContext *c, 
>> AVIOContext *pb, MOVAtom atom)
>>       AVStream *st = NULL;
>>       FFStream *sti = NULL;
>>       MOVStreamContext *sc;
>> -    MOVStts *ctts_data;
>> +    MOVCtts *ctts_data;
>>       uint64_t offset;
>>       int64_t dts, pts = AV_NOPTS_VALUE;
>>       int data_offset = 0;
>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> index 37d4403f7a..233aee62d4 100644
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -2434,7 +2434,7 @@ static int mov_write_stsd_tag(AVFormatContext 
>> *s, AVIOContext *pb, MOVMuxContext
>>   static int mov_write_ctts_tag(AVFormatContext *s, AVIOContext *pb, 
>> MOVTrack *track)
>>   {
>>       MOVMuxContext *mov = s->priv_data;
>> -    MOVStts *ctts_entries;
>> +    MOVCtts *ctts_entries;
>>       uint32_t entries = 0;
>>       uint32_t atom_size;
>>       int i;
>
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavformat/isom.h b/libavformat/isom.h
index f3c18c95be..ef8f19b18c 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -55,9 +55,14 @@  struct AVAESCTR;
 
 typedef struct MOVStts {
     unsigned int count;
-    int duration;
+    unsigned int duration;
 } MOVStts;
 
+typedef struct MOVCtts {
+    unsigned int count;
+    int duration;
+} MOVCtts;
+
 typedef struct MOVStsc {
     int first;
     int count;
@@ -168,7 +173,7 @@  typedef struct MOVStreamContext {
     uint8_t *sdtp_data;
     unsigned int ctts_count;
     unsigned int ctts_allocated_size;
-    MOVStts *ctts_data;
+    MOVCtts *ctts_data;
     unsigned int stsc_count;
     MOVStsc *stsc_data;
     unsigned int stsc_index;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 8a910a3165..451cb78bbf 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -77,7 +77,7 @@  typedef struct MOVParseTableEntry {
 
 static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom);
 static int mov_read_mfra(MOVContext *c, AVIOContext *f);
-static int64_t add_ctts_entry(MOVStts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size,
+static int64_t add_ctts_entry(MOVCtts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size,
                               int count, int duration);
 
 static int mov_metadata_track_or_disc_number(MOVContext *c, AVIOContext *pb,
@@ -2938,7 +2938,7 @@  static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         return AVERROR(ENOMEM);
 
     for (i = 0; i < entries && !pb->eof_reached; i++) {
-        int sample_duration;
+        unsigned int sample_duration;
         unsigned int sample_count;
         unsigned int min_entries = FFMIN(FFMAX(i + 1, 1024 * 1024), entries);
         MOVStts *stts_data = av_fast_realloc(sc->stts_data, &alloc_size,
@@ -3191,7 +3191,7 @@  static int get_edit_list_entry(MOVContext *mov,
 static int find_prev_closest_index(AVStream *st,
                                    AVIndexEntry *e_old,
                                    int nb_old,
-                                   MOVStts* ctts_data,
+                                   MOVCtts* ctts_data,
                                    int64_t ctts_count,
                                    int64_t timestamp_pts,
                                    int flag,
@@ -3342,17 +3342,17 @@  static void fix_index_entry_timestamps(AVStream* st, int end_index, int64_t end_
  * Append a new ctts entry to ctts_data.
  * Returns the new ctts_count if successful, else returns -1.
  */
-static int64_t add_ctts_entry(MOVStts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size,
+static int64_t add_ctts_entry(MOVCtts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size,
                               int count, int duration)
 {
-    MOVStts *ctts_buf_new;
-    const size_t min_size_needed = (*ctts_count + 1) * sizeof(MOVStts);
+    MOVCtts *ctts_buf_new;
+    const size_t min_size_needed = (*ctts_count + 1) * sizeof(MOVCtts);
     const size_t requested_size =
         min_size_needed > *allocated_size ?
         FFMAX(min_size_needed, 2 * (*allocated_size)) :
         min_size_needed;
 
-    if ((unsigned)(*ctts_count) >= UINT_MAX / sizeof(MOVStts) - 1)
+    if ((unsigned)(*ctts_count) >= UINT_MAX / sizeof(MOVCtts) - 1)
         return -1;
 
     ctts_buf_new = av_fast_realloc(*ctts_data, allocated_size, requested_size);
@@ -3486,7 +3486,7 @@  static void mov_fix_index(MOVContext *mov, AVStream *st)
     int nb_old = sti->nb_index_entries;
     const AVIndexEntry *e_old_end = e_old + nb_old;
     const AVIndexEntry *current = NULL;
-    MOVStts *ctts_data_old = msc->ctts_data;
+    MOVCtts *ctts_data_old = msc->ctts_data;
     int64_t ctts_index_old = 0;
     int64_t ctts_sample_old = 0;
     int64_t ctts_count_old = msc->ctts_count;
@@ -3793,7 +3793,7 @@  static void mov_build_index(MOVContext *mov, AVStream *st)
     unsigned int stps_index = 0;
     unsigned int i, j;
     uint64_t stream_size = 0;
-    MOVStts *ctts_data_old = sc->ctts_data;
+    MOVCtts *ctts_data_old = sc->ctts_data;
     unsigned int ctts_count_old = sc->ctts_count;
 
     if (sc->elst_count) {
@@ -4754,7 +4754,7 @@  static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     AVStream *st = NULL;
     FFStream *sti = NULL;
     MOVStreamContext *sc;
-    MOVStts *ctts_data;
+    MOVCtts *ctts_data;
     uint64_t offset;
     int64_t dts, pts = AV_NOPTS_VALUE;
     int data_offset = 0;
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 37d4403f7a..233aee62d4 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2434,7 +2434,7 @@  static int mov_write_stsd_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContext
 static int mov_write_ctts_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track)
 {
     MOVMuxContext *mov = s->priv_data;
-    MOVStts *ctts_entries;
+    MOVCtts *ctts_entries;
     uint32_t entries = 0;
     uint32_t atom_size;
     int i;